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

lambda - Add support for purge_tags #1202

Conversation

tremble
Copy link
Contributor

@tremble tremble commented Jun 2, 2022

SUMMARY
  • Use tagging fragment
  • Add purge_tags
  • Add resource_tags alias to tags
  • fix bug with returned tag names getting snake cased
  • fix bug where the lambda module was modifying tags in check mode
  • Tweak tagging to require an explicit tags: {} to remove tags
ISSUE TYPE
  • Bugfix Pull Request
  • Docs Pull Request
  • Feature Pull Request
COMPONENT NAME

lambda

ADDITIONAL INFORMATION

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_triage plugins plugin (any type) tests tests labels Jun 2, 2022
@github-actions
Copy link

github-actions bot commented Jun 2, 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 FAILURE in 4m 36s (non-voting)
✔️ build-ansible-collection SUCCESS in 8m 19s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 57s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 58s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 15m 49s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 17m 05s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 34s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 5m 52s
✔️ ansible-test-splitter SUCCESS in 3m 28s
✔️ integration-community.aws-1 SUCCESS in 8m 59s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

# We're done
module.exit_json(changed=changed, **camel_dict_to_snake_dict(response))
module.exit_json(changed=changed, **response)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we axe the format_response method by using ignore_list for the tags?

Suggested change
module.exit_json(changed=changed, **response)
module.exit_json(changed=changed, **camel_dict_to_snake_dict(response, ignore_list=['Tags'])

Copy link
Contributor Author

@tremble tremble Jun 3, 2022

Choose a reason for hiding this comment

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

In theory, yes.

However, I prefer splitting it out because we have two places where we're making the same transformation. By turning it into a function we reduce the chance of introducing inconsistent behaviour in a future change.

For example: When making this change I initially missed that this transformation was happening in two different spots!

Copy link
Contributor

@jatorcasso jatorcasso left a comment

Choose a reason for hiding this comment

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

Left a minor comment but otherwise LGTM

@tremble tremble added the mergeit Merge the PR (SoftwareFactory) label Jun 3, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 5m 17s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 54s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 31s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 9m 55s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 12m 57s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 13m 13s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 45s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 23m 07s
✔️ ansible-test-units-community-aws-python39 SUCCESS in 6m 29s
✔️ ansible-test-splitter SUCCESS in 2m 21s
✔️ integration-community.aws-1 SUCCESS in 8m 51s
⚠️ integration-community.aws-2 SKIPPED
⚠️ integration-community.aws-3 SKIPPED
⚠️ integration-community.aws-4 SKIPPED
⚠️ integration-community.aws-5 SKIPPED
⚠️ integration-community.aws-6 SKIPPED
⚠️ integration-community.aws-7 SKIPPED
⚠️ integration-community.aws-8 SKIPPED
⚠️ integration-community.aws-9 SKIPPED
⚠️ integration-community.aws-10 SKIPPED
⚠️ integration-community.aws-11 SKIPPED
⚠️ integration-community.aws-12 SKIPPED
⚠️ integration-community.aws-13 SKIPPED

@softwarefactory-project-zuul softwarefactory-project-zuul bot merged commit 982580a into ansible-collections:main Jun 3, 2022
@tremble tremble deleted the tagging/purge/lambda branch July 7, 2022 19:23
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
lambda - Add support for purge_tags

SUMMARY

Use tagging fragment
Add purge_tags
Add resource_tags alias to tags
fix bug with returned tag names getting snake cased
fix bug where the lambda module was modifying tags in check mode
Tweak tagging to require an explicit tags: {} to remove tags

ISSUE TYPE

Bugfix Pull Request
Docs Pull Request
Feature Pull Request

COMPONENT NAME
lambda
ADDITIONAL INFORMATION

Reviewed-by: Joseph Torcasso <None>
Reviewed-by: Mark Chappell <None>

This commit was initially merged in https://github.com/ansible-collections/community.aws
See: ansible-collections@982580a
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
add pytest-forked to test requirements

SUMMARY
ansible-test is using pytest --forked we need the sub-module installed
ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
test-requirements.txt
ADDITIONAL INFORMATION
See also: ansible/ansible#76679

Reviewed-by: Alina Buzachis <None>
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…s#1199)

ec2_instance - validate options on tower_callback

Depends-On: ansible-collections#1202
SUMMARY

Validate options for tower_callback parameter
Set tower_callback.set_password (the password) to no_log=True

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
ec2_instance
ADDITIONAL INFORMATION

Reviewed-by: Alina Buzachis <None>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug community_review integration tests/integration mergeit Merge the PR (SoftwareFactory) module module needs_triage plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants