Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update return block of Backup modules #2106

Conversation

mandar242
Copy link
Contributor

SUMMARY

Updated return block of following modules, tried to ensure consistency across their return blocks

1. backup_plan
2. backup_plan_info
3. backup_vault
4. backup_selection
ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME
ADDITIONAL INFORMATION

Copy link

github-actions bot commented May 21, 2024

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

This comment was marked as outdated.

@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from b642a08 to 70a8048 Compare May 22, 2024 01:23

This comment was marked as outdated.

plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_vault.py Outdated Show resolved Hide resolved
@gundalow
Copy link
Contributor

Will this fix the syntax error in the current backup_plan module docs
https://galaxy.ansible.com/ui/repo/published/amazon/aws/content/module/backup_plan/

@alinabuzachis
Copy link
Collaborator

Will this fix the syntax error in the current backup_plan module docs https://galaxy.ansible.com/ui/repo/published/amazon/aws/content/module/backup_plan/

@mandar242 Can you also please have a look ^ ?

@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from 70a8048 to ced7882 Compare May 22, 2024 16:07
@mandar242
Copy link
Contributor Author

Will this fix the syntax error in the current backup_plan module docs https://galaxy.ansible.com/ui/repo/published/amazon/aws/content/module/backup_plan/

@alinabuzachis @gundalow I am not sure, looks like the same error has been there since 7.0.0, I'll dig into it and get back

This comment was marked as outdated.

@mandar242
Copy link
Contributor Author

Will this fix the syntax error in the current backup_plan module docs https://galaxy.ansible.com/ui/repo/published/amazon/aws/content/module/backup_plan/

Not sure what's the exact issue, ansible-doc -j amazon.aws.backup_plan works fine for me locally, tried on stable-8 and stable-7`

% ansible-doc -j amazon.aws.backup_plan                                        
{
    "amazon.aws.backup_plan": {
        "doc": {
            "author": [
                "Kristof Imre Szabo (@krisek)",
                "Alina Buzachis (@alinabuzachis)",
                "Helen Bailey (@hakbailey)"
            ],
            "collection": "amazon.aws",
            "description": [
                "Creates, updates, or deletes AWS Backup Plans",
                "For more information see the AWS documentation for Backup plans U(https://docs.aws.amazon.com/aws-backup/latest/devguide/about-backup-plans.html)."
               ...

This comment was marked as outdated.

@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from b5a84ce to f382af7 Compare May 22, 2024 20:48
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/0f97f15c019f45138c0a81ed8f4f3015

✔️ ansible-galaxy-importer SUCCESS in 5m 00s
✔️ build-ansible-collection SUCCESS in 18m 17s
✔️ ansible-test-splitter SUCCESS in 7m 16s
✔️ integration-amazon.aws-1 SUCCESS in 10m 33s
✔️ integration-amazon.aws-2 SUCCESS in 7m 28s
Skipped 42 jobs

@@ -33,7 +33,7 @@
rules:
description:
- An array of BackupRule objects, each of which specifies a scheduled task that is used to back up a selection of resources.
- Required when O(state=present).
- Required when o(state=present).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- Required when o(state=present).
- Required when O(state=present).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alinabuzachis this change is causing sanity tests to fail, hence went with o instead of O. same for other suggestions

Running sanity test "validate-modules"
ERROR: Found 1 validate-modules issue(s) which need to be resolved:
ERROR: plugins/modules/backup_plan_info.py:0:0: invalid-documentation-markup: Directive "O(rules.lifecycle)" contains a non-existing option "rules.lifecycle"
See documentation for help: https://docs.ansible.com/ansible-core/2.15/dev_guide/testing/sanity/validate-modules.html
Running sanity test "yamllint"
FATAL: The 1 sanity test(s) listed below (out of 46) failed. See error output above for details.
validate-modules

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mandar242 The error you're seeing is because the module does not implement that option or the path to that option is wrong. In your case you could either:

  • To reference an option for another plugin/module plugin.fqcn.name of type type, use O(plugin.fqcn.name#type:option) and O(plugin.fqcn.name#type:option=name). For modules, use type=module. The FQCN and plugin type can be ignored by the documentation renderer, turned into a link to that plugin, or even directly to the option of that plugin. https://docs.ansible.com/ansible/latest/dev_guide/developing_modules_documenting.html#semantic-markup-within-module-documentation If you want to refer to when that option is set in the backup_plan module. Probably something like returned: when O(amazon.aws.backup_plan#module:rules.lifecycle) configured

  • Use RV() to refer the returned value. In your case it will be RV(backup_plans.backup_plan.rules.lifecycle).

  • Use simply when configured.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @alinabuzachis , this helped

plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan_info.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan_info.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan_info.py Outdated Show resolved Hide resolved
@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from 5e473a1 to 4bf72ef Compare May 23, 2024 20:31
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/aafaee8aa7d8429383a03b009ae39629

✔️ ansible-galaxy-importer SUCCESS in 5m 35s
✔️ build-ansible-collection SUCCESS in 18m 17s
✔️ ansible-test-splitter SUCCESS in 8m 00s
✔️ integration-amazon.aws-1 SUCCESS in 6m 26s
✔️ integration-amazon.aws-2 SUCCESS in 7m 49s
Skipped 42 jobs

@gundalow
Copy link
Contributor

Will this fix the syntax error in the current backup_plan module docs https://galaxy.ansible.com/ui/repo/published/amazon/aws/content/module/backup_plan/

Not sure what's the exact issue, ansible-doc -j amazon.aws.backup_plan works fine for me locally, tried on stable-8 and stable-7`

% ansible-doc -j amazon.aws.backup_plan                                        
{
    "amazon.aws.backup_plan": {
        "doc": {
            "author": [
                "Kristof Imre Szabo (@krisek)",
                "Alina Buzachis (@alinabuzachis)",
                "Helen Bailey (@hakbailey)"
            ],
            "collection": "amazon.aws",
            "description": [
                "Creates, updates, or deletes AWS Backup Plans",
                "For more information see the AWS documentation for Backup plans U(https://docs.aws.amazon.com/aws-backup/latest/devguide/about-backup-plans.html)."
               ...

Can you please run ansible-test sanity inside this collection, thanks

@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from 4bf72ef to 983a598 Compare May 28, 2024 19:40
@mandar242
Copy link
Contributor Author

Will this fix the syntax error in the current backup_plan module docs https://galaxy.ansible.com/ui/repo/published/amazon/aws/content/module/backup_plan/

Not sure what's the exact issue, ansible-doc -j amazon.aws.backup_plan works fine for me locally, tried on stable-8 and stable-7`

% ansible-doc -j amazon.aws.backup_plan                                        
{
    "amazon.aws.backup_plan": {
        "doc": {
            "author": [
                "Kristof Imre Szabo (@krisek)",
                "Alina Buzachis (@alinabuzachis)",
                "Helen Bailey (@hakbailey)"
            ],
            "collection": "amazon.aws",
            "description": [
                "Creates, updates, or deletes AWS Backup Plans",
                "For more information see the AWS documentation for Backup plans U(https://docs.aws.amazon.com/aws-backup/latest/devguide/about-backup-plans.html)."
               ...

Can you please run ansible-test sanity inside this collection, thanks

Hi @gundalow , the syntax issue in https://galaxy.ansible.com/ui/repo/published/amazon/aws/content/module/backup_plan/
should likley get resolved once the issue is fixed #2110

Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/5b54742e41054686a4ca99c79994926f

✔️ ansible-galaxy-importer SUCCESS in 4m 59s
✔️ build-ansible-collection SUCCESS in 17m 30s
✔️ ansible-test-splitter SUCCESS in 6m 39s
✔️ integration-amazon.aws-1 SUCCESS in 10m 36s
✔️ integration-amazon.aws-2 SUCCESS in 9m 59s
Skipped 42 jobs

@mandar242 mandar242 requested a review from alinabuzachis May 28, 2024 20:30
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/9accfc6a8c184a18bc9b7819d522bdc0

✔️ ansible-galaxy-importer SUCCESS in 5m 40s
✔️ build-ansible-collection SUCCESS in 16m 29s
✔️ ansible-test-splitter SUCCESS in 8m 36s
✔️ integration-amazon.aws-1 SUCCESS in 6m 53s
✔️ integration-amazon.aws-2 SUCCESS in 7m 15s
Skipped 42 jobs

@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from bbb8fa2 to a04d9ce Compare May 30, 2024 22:23
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/b1ce41ea5c4747d99d6a74c5f3c348eb

✔️ ansible-galaxy-importer SUCCESS in 4m 24s
✔️ build-ansible-collection SUCCESS in 17m 36s
✔️ ansible-test-splitter SUCCESS in 9m 33s
✔️ integration-amazon.aws-1 SUCCESS in 6m 36s
✔️ integration-amazon.aws-2 SUCCESS in 7m 47s
Skipped 42 jobs

@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from a04d9ce to 8f8ac24 Compare June 4, 2024 19:05
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/4c6afe9c56004ae2af47e979cddb1e93

✔️ ansible-galaxy-importer SUCCESS in 4m 58s
✔️ build-ansible-collection SUCCESS in 19m 38s
✔️ ansible-test-splitter SUCCESS in 6m 11s
✔️ integration-amazon.aws-1 SUCCESS in 6m 19s
✔️ integration-amazon.aws-2 SUCCESS in 7m 59s
Skipped 42 jobs

plugins/modules/backup_vault.py Outdated Show resolved Hide resolved
plugins/modules/backup_vault.py Outdated Show resolved Hide resolved
plugins/modules/backup_selection.py Outdated Show resolved Hide resolved
plugins/modules/backup_selection.py Outdated Show resolved Hide resolved
plugins/modules/backup_selection.py Outdated Show resolved Hide resolved
plugins/modules/backup_selection.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan.py Outdated Show resolved Hide resolved
plugins/modules/backup_plan_info.py Outdated Show resolved Hide resolved
@mandar242 mandar242 force-pushed the backup_modules_return_block_updates branch from 4f5171d to b50804d Compare June 7, 2024 20:11
Copy link
Contributor

Build succeeded.
https://ansible.softwarefactory-project.io/zuul/buildset/7b85f14a87d542f58bcbbaab7fca63b3

✔️ ansible-galaxy-importer SUCCESS in 7m 06s
✔️ build-ansible-collection SUCCESS in 22m 39s
✔️ ansible-test-splitter SUCCESS in 6m 44s
✔️ integration-amazon.aws-1 SUCCESS in 9m 28s
✔️ integration-amazon.aws-2 SUCCESS in 9m 27s
Skipped 42 jobs

@mandar242 mandar242 added the mergeit Merge the PR (SoftwareFactory) label Jun 7, 2024
Copy link
Contributor

Build succeeded (gate pipeline).
https://ansible.softwarefactory-project.io/zuul/buildset/127175a75d8c49ebb67280093fc41f5c

✔️ ansible-galaxy-importer SUCCESS in 4m 14s
✔️ build-ansible-collection SUCCESS in 17m 10s
✔️ ansible-test-splitter SUCCESS in 6m 41s
✔️ integration-amazon.aws-1 SUCCESS in 6m 14s
✔️ integration-amazon.aws-2 SUCCESS in 8m 02s
Skipped 42 jobs

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 7c888ce into ansible-collections:main Jun 7, 2024
40 checks passed
alinabuzachis pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Jun 26, 2024
Update return block of Backup modules

SUMMARY

Updated return block of following modules, tried to ensure consistency across their return blocks
1. backup_plan
2. backup_plan_info
3. backup_vault
4. backup_selection


ISSUE TYPE


Docs Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
Reviewed-by: Mandar Kulkarni <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Mike Graves <[email protected]>
@tremble tremble added the backport-8 PR should be backported to the stable-8 branch label Aug 27, 2024
Copy link

patchback bot commented Aug 27, 2024

Backport to stable-8: 💔 cherry-picking failed — conflicts found

❌ Failed to cleanly apply 7c888ce on top of patchback/backports/stable-8/7c888ce60346e4d9bdbe425038f9b82bf9ec27ff/pr-2106

Backporting merged PR #2106 into main

  1. Ensure you have a local repo clone of your fork. Unless you cloned it
    from the upstream, this would be your origin remote.
  2. Make sure you have an upstream repo added as a remote too. In these
    instructions you'll refer to it by the name upstream. If you don't
    have it, here's how you can add it:
    $ git remote add upstream https://github.com/ansible-collections/amazon.aws.git
  3. Ensure you have the latest copy of upstream and prepare a branch
    that will hold the backported code:
    $ git fetch upstream
    $ git checkout -b patchback/backports/stable-8/7c888ce60346e4d9bdbe425038f9b82bf9ec27ff/pr-2106 upstream/stable-8
  4. Now, cherry-pick PR Update return block of Backup modules #2106 contents into that branch:
    $ git cherry-pick -x 7c888ce60346e4d9bdbe425038f9b82bf9ec27ff
    If it'll yell at you with something like fatal: Commit 7c888ce60346e4d9bdbe425038f9b82bf9ec27ff is a merge but no -m option was given., add -m 1 as follows instead:
    $ git cherry-pick -m1 -x 7c888ce60346e4d9bdbe425038f9b82bf9ec27ff
  5. At this point, you'll probably encounter some merge conflicts. You must
    resolve them in to preserve the patch from PR Update return block of Backup modules #2106 as close to the
    original as possible.
  6. Push this branch to your fork on GitHub:
    $ git push origin patchback/backports/stable-8/7c888ce60346e4d9bdbe425038f9b82bf9ec27ff/pr-2106
  7. Create a PR, ensure that the CI is green. If it's not — update it so that
    the tests and any other checks pass. This is it!
    Now relax and wait for the maintainers to process your pull request
    when they have some cycles to do reviews. Don't worry — they'll tell you if
    any improvements are necessary when the time comes!

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@tremble tremble added the backport_failed Backport failed, needs review label Aug 27, 2024
Copy link

patchback bot commented Aug 27, 2024

Backport to stable-failed: 💔 cherry-picking failed — target branch does not exist

❌ Failed to find branch stable-failed

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@tremble tremble added backport-8 PR should be backported to the stable-8 branch and removed backport-8 PR should be backported to the stable-8 branch labels Aug 27, 2024
Copy link

patchback bot commented Aug 27, 2024

Backport to stable-8: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-8/7c888ce60346e4d9bdbe425038f9b82bf9ec27ff/pr-2106

Backported as #2249

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Aug 27, 2024
Update return block of Backup modules

SUMMARY

Updated return block of following modules, tried to ensure consistency across their return blocks
1. backup_plan
2. backup_plan_info
3. backup_vault
4. backup_selection

ISSUE TYPE

Docs Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis
Reviewed-by: Mandar Kulkarni <[email protected]>
Reviewed-by: Bikouo Aubin
Reviewed-by: Mike Graves <[email protected]>
(cherry picked from commit 7c888ce)
@tremble tremble removed the backport_failed Backport failed, needs review label Aug 27, 2024
softwarefactory-project-zuul bot pushed a commit that referenced this pull request Aug 27, 2024
This is a backport of PR #2106 as merged into main (7c888ce).
SUMMARY

Updated return block of following modules, tried to ensure consistency across their return blocks
1. backup_plan
2. backup_plan_info
3. backup_vault
4. backup_selection


ISSUE TYPE


Docs Pull Request

COMPONENT NAME

ADDITIONAL INFORMATION

Reviewed-by: Mark Chappell
@mandar242 mandar242 deleted the backup_modules_return_block_updates branch October 29, 2024 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8 PR should be backported to the stable-8 branch mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants