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

[PR #1115/f2ad6375 backport][stable-3] execute_lambda - fix check mode and update RETURN docs #1136

Conversation

patchback[bot]
Copy link

@patchback patchback bot commented May 7, 2022

This is a backport of PR #1115 as merged into main (f2ad637).

Depends-On: #1116

SUMMARY
  • check_mode fix
  • update RETURN docs to match what is actually being returned
  • require one of name, function_arn
ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

execute_lambda

ADDITIONAL INFORMATION

I noticed some modules in community.aws will return data directly, and others will return data nested in a dict.

Example: let iam_group be the module, retrieving a key called group_arn, and registering the response as response. Some modules you would need to query result.iam_group.group_arn, meanwhile in others, you can result.group_arn (where iam_group is assumed, since its the name of the module). Do we have a preference for either method? Should we come to some sort of collection-wide consensus on which to use moving forward?

execute_lambda - fix check mode and update RETURN docs

Depends-On: #1116
SUMMARY

check_mode fix
update RETURN docs to match what is actually being returned
require one of name, function_arn

ISSUE TYPE

Bugfix Pull Request

COMPONENT NAME
execute_lambda
ADDITIONAL INFORMATION
I noticed some modules in community.aws will return data directly, and others will return data nested in a dict.
Example: let iam_group be the module, retrieving a key called group_arn, and registering the response as response. Some modules you would need to query result.iam_group.group_arn, meanwhile in others, you can result.group_arn (where iam_group is assumed, since its the name of the module). Do we have a preference for either method? Should we come to some sort of collection-wide consensus on which to use moving forward?

Reviewed-by: Sloane Hertel <None>
Reviewed-by: Markus Bergholz <[email protected]>
(cherry picked from commit f2ad637)
@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review integration tests/integration module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) tests tests labels May 7, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded.

ansible-galaxy-importer FAILURE in 4m 59s (non-voting)
✔️ build-ansible-collection SUCCESS in 5m 26s
✔️ ansible-test-sanity-docker-devel SUCCESS in 10m 25s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 01s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 13m 55s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 10m 00s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 11m 05s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 32s
✔️ ansible-test-splitter SUCCESS in 3m 32s
✔️ integration-community.aws-1 SUCCESS in 8m 01s
⚠️ 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

@markuman markuman added the mergeit Merge the PR (SoftwareFactory) label May 9, 2022
@softwarefactory-project-zuul
Copy link
Contributor

Build succeeded (gate pipeline).

ansible-galaxy-importer FAILURE in 4m 25s (non-voting)
✔️ build-ansible-collection SUCCESS in 4m 46s
✔️ ansible-test-sanity-docker-devel SUCCESS in 11m 39s (non-voting)
✔️ ansible-test-sanity-docker-milestone SUCCESS in 10m 25s
✔️ ansible-test-sanity-docker-stable-2.9 SUCCESS in 11m 31s
✔️ ansible-test-sanity-docker-stable-2.11 SUCCESS in 11m 53s
✔️ ansible-test-sanity-docker-stable-2.12 SUCCESS in 10m 33s
✔️ ansible-test-units-community-aws-python38 SUCCESS in 6m 29s
✔️ ansible-test-splitter SUCCESS in 3m 58s
✔️ integration-community.aws-1 SUCCESS in 8m 24s
⚠️ 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 1e1ee47 into stable-3 May 9, 2022
@softwarefactory-project-zuul softwarefactory-project-zuul bot deleted the patchback/backports/stable-3/f2ad6375ad5ff0d07158f7b8248968b1d5e08966/pr-1115 branch May 9, 2022 17:21
abikouo pushed a commit to abikouo/community.aws that referenced this pull request Oct 24, 2023
…ble_policy slightly (ansible-collections#1136)

Improve unit test coverage of module_utils.policy

SUMMARY

Improve unit test coverage of module_utils.policy
Slight refactor of _hashable_policy to reduce complexity (and make testing easier)
Deprecates sort_json_policy_dict, it's very naive and was only used by ecs_ecr

ISSUE TYPE

Feature Pull Request

COMPONENT NAME
plugins/module_utils/policy.py
ADDITIONAL INFORMATION

Reviewed-by: Gonéri Le Bouder <[email protected]>
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 new_contributor Help guide this first time contributor plugins plugin (any type) tests tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants