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 dev guidelines - add return values guidelines #865

Conversation

jatorcasso
Copy link
Contributor

@jatorcasso jatorcasso commented Jun 7, 2022

SUMMARY

As discussed in the last working group meeting. Updating the guidelines to create standards for what _info modules should return.

Items addressed:

  • more info on how to return tags using boto3_tag_list_to_ansible_dict
  • info modules should return list of dicts
  • how to deprecate keys
ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

docs/docsite/rst/dev_guidelines.rst

@jatorcasso jatorcasso requested a review from mandar242 June 7, 2022 02:45
@jatorcasso jatorcasso marked this pull request as draft June 7, 2022 02:45
@github-actions
Copy link

github-actions bot commented Jun 7, 2022

Docs Build 📝

Thank you for contribution!✨

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

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 17s
✔️ build-ansible-collection SUCCESS in 5m 06s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 12s
✔️ ansible-test-sanity-aws-ansible-2.9-python38 SUCCESS in 10m 55s
✔️ ansible-test-sanity-aws-ansible-2.11-python38 SUCCESS in 9m 39s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 57s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 20s

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Thanks for this, definitely a good start, I think we definitely need something about using dicts for tags rather than the boto3 style list of dictionaries. We should also mention the use of snake case for keys rather than camel case.

docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 04s
✔️ build-ansible-collection SUCCESS in 4m 43s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 03s
✔️ ansible-test-sanity-aws-ansible-2.9-python38 SUCCESS in 11m 34s
✔️ ansible-test-sanity-aws-ansible-2.11-python38 SUCCESS in 10m 31s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 20s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 21s

Copy link
Contributor

@tremble tremble left a comment

Choose a reason for hiding this comment

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

Looking good, a couple of nits I happened to notice.

docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 05s
✔️ build-ansible-collection SUCCESS in 4m 39s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 04s
✔️ ansible-test-sanity-aws-ansible-2.9-python38 SUCCESS in 10m 17s
✔️ ansible-test-sanity-aws-ansible-2.11-python38 SUCCESS in 9m 33s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 24s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 02s

@tremble tremble changed the title WIP - *_info modules return values - Update dev guidelines [WIP] *_info modules return values - Update dev guidelines Jun 24, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Merge Failed.

This change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.
Warning:
Error merging github.com/ansible-collections/amazon.aws for 865,ab364bc8eb57be8eec7fe2b1c3c959f338daabb6

@jatorcasso jatorcasso force-pushed the return_vals/docs_fragment branch from ab364bc to 03e223c Compare June 27, 2022 00:01
@jatorcasso jatorcasso changed the title [WIP] *_info modules return values - Update dev guidelines *_info modules return values - Update dev guidelines Jun 27, 2022
@jatorcasso jatorcasso marked this pull request as ready for review June 27, 2022 00:03
@ansibullbot ansibullbot added community_review and removed WIP Work in progress labels Jun 27, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 10m 31s
✔️ build-ansible-collection SUCCESS in 9m 42s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 22m 45s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 18m 52s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 22m 58s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 20m 31s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 21m 37s

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 29s
✔️ build-ansible-collection SUCCESS in 4m 47s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 10m 47s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 44s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 10m 15s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 54s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 42s

Copy link
Collaborator

@jillr jillr left a comment

Choose a reason for hiding this comment

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

+1 the substance of the change, if we can get a docs review for any prose recommendations before merging that would be ideal.

Copy link

@Dule-martins Dule-martins left a comment

Choose a reason for hiding this comment

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

Those were my thought after going through the PR Docs.

docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
docs/docsite/rst/dev_guidelines.rst Outdated Show resolved Hide resolved
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 03s
✔️ build-ansible-collection SUCCESS in 5m 24s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 16m 55s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 15m 12s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 17m 01s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 5m 51s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 00s

@tremble tremble changed the title *_info modules return values - Update dev guidelines Update dev guidelines - add return values guidelines Jul 6, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 4m 54s
✔️ build-ansible-collection SUCCESS in 5m 02s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 55s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 42s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 8m 55s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 8m 45s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 9m 53s

@jatorcasso jatorcasso added the mergeit Merge the PR (SoftwareFactory) label Jul 8, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 3m 36s
✔️ build-ansible-collection SUCCESS in 5m 15s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 11m 23s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 10m 26s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 11m 19s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 35s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 32s

@softwarefactory-project-zuul
Copy link
Contributor

Pull request merge failed: Resource not accessible by integration, You may need to manually rebase your PR and retry.

@jatorcasso jatorcasso force-pushed the return_vals/docs_fragment branch from 4e2014d to 209756b Compare July 8, 2022 20:40
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

✔️ ansible-galaxy-importer SUCCESS in 3m 56s
✔️ build-ansible-collection SUCCESS in 4m 46s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 53s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 10m 00s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 9m 48s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 53s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 5m 52s

@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

✔️ ansible-galaxy-importer SUCCESS in 4m 27s
✔️ build-ansible-collection SUCCESS in 5m 29s
✔️ ansible-test-sanity-aws-ansible-python38 SUCCESS in 9m 46s
✔️ ansible-test-sanity-aws-ansible-2.12-python38 SUCCESS in 9m 13s
✔️ ansible-test-sanity-aws-ansible-2.13-python38 SUCCESS in 10m 01s
✔️ ansible-test-units-amazon-aws-python38 SUCCESS in 6m 58s
✔️ ansible-test-units-amazon-aws-python39 SUCCESS in 6m 13s

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 7f4b4a3 into ansible-collections:main Jul 8, 2022
alinabuzachis pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 16, 2022
…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@ca1d33f
goneri pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 21, 2022
…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@ca1d33f
goneri pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 21, 2022
…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@ca1d33f
goneri pushed a commit to alinabuzachis/amazon.aws that referenced this pull request Sep 21, 2022
…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections/community.aws@ca1d33f
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false 
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Sep 18, 2023
…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false 
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>
abikouo pushed a commit to abikouo/amazon.aws that referenced this pull request Oct 24, 2023
…collections#865)

Add AWSRetry backoff logic to route53_zone and route53_info

SUMMARY
Add AWSRetry backoff logic to route53_zone and route53_info. Currently from time to time I've been hitting AWS throttling errors leading to ansible failures:
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: botocore.exceptions.ClientError: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded
fatal: [localhost_staging -> 127.0.0.1]: FAILED! => changed=false 
  boto3_version: 1.20.34
  botocore_version: 1.23.34
  error:
    code: Throttling
    message: Rate exceeded
    type: Sender
  msg: 'Could not list current hosted zones: An error occurred (Throttling) when calling the ListHostedZones operation (reached max retries: 4): Rate exceeded'
  response_metadata:
    http_headers:
      connection: close
      content-length: '255'
      content-type: text/xml
      date: Fri, 14 Jan 2022 12:09:35 GMT
      x-amzn-requestid: xxxxxxx
    http_status_code: 400
    max_attempts_reached: true
    request_id: xxxxxxx
    retry_attempts: 4
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
route53_zone
route53_info
ADDITIONAL INFORMATION
I've added the standard backoff retry logic and split out the paginators.

Reviewed-by: Alina Buzachis <None>
Reviewed-by: Markus Bergholz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community_review docs mergeit Merge the PR (SoftwareFactory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants