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

Fix UnboundLocalError in sqs_queue #389

Merged
merged 3 commits into from
Mar 16, 2021

Conversation

leedm777
Copy link
Contributor

@leedm777 leedm777 commented Feb 3, 2021

SUMMARY

The variable existing_value is nowhere to be found, but looks like
this might have been missed in a rename. Changing to value.

Fixes #172

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

sqs_queue

ADDITIONAL INFORMATION

See details in #172

@ansibullbot
Copy link

@ansibullbot ansibullbot added bug This issue/PR relates to a bug community_review module module needs_triage new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review labels Feb 3, 2021
@cmpsoares91
Copy link

Hello,

When can we expect this merge to happen?

Kind regards,
Carlos Manuel Soares.

@jillr
Copy link
Collaborator

jillr commented Feb 12, 2021

Hi @leedm777, thanks very much for this patch. It looks like this bug was missed because we don't have any integration tests that exercise update_sqs_queue. Would you be able to add some tests (ansible tasks) to tests/integration/targets/sqs_queue/tasks/main.yml that test update functionality?

@leedm777
Copy link
Contributor Author

@jillr ,

Would you be able to add some tests (ansible tasks) to tests/integration/targets/sqs_queue/tasks/main.yml that test update functionality?

Absolutely. I don't see any instructions on how to run the tests; can you point me in the right direction? Once I can run them, then I'll be able to write some new tests😄

@jillr
Copy link
Collaborator

jillr commented Feb 12, 2021

You can always just push the changes to the PR and CI will run them for you, if you want to use you own account locally you can run the integration test locally by copying this config file https://github.com/ansible/ansible/blob/devel/test/lib/ansible_test/config/cloud-config-aws.ini.template to tests/integration/cloud-config-aws.ini (we have a .gitignore entry for that filename but still be careful of credentials files). Then you can run:
ansible-test integration cloudwatchevent_rule --docker to pull the test container and run the test against your own AWS account.

leedm777 and others added 3 commits March 16, 2021 12:45
The variable `existing_value` is nowhere to be found, but looks like
this might have been missed in a rename. Changing to `value`.

Fixes ansible-collections#172
@tremble tremble force-pushed the fix-sqs-queue-update branch from aa8cbff to 5d2a484 Compare March 16, 2021 11:46
@ansibullbot ansibullbot added integration tests/integration tests tests and removed small_patch Hopefully easy to review labels Mar 16, 2021
@tremble
Copy link
Contributor

tremble commented Mar 16, 2021

I've added a minimal integration test and a changelog.

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.

change LGTM.

The test's straight out of the examples and I've tested locally with and without the change. Assuming all tests pass let's get this merged.

@tremble tremble merged commit e892ab3 into ansible-collections:main Mar 16, 2021
@tremble
Copy link
Contributor

tremble commented Mar 16, 2021

@leedm777,

Thanks for your contribution. I'm sorry it took so long to get this PR merged. The fix should be available in the next versions of Ansible 2.10 and this collection.

danquixote pushed a commit to danquixote/community.aws that referenced this pull request May 16, 2021
* Fix UnboundLocalError in sqs_queue
The variable `existing_value` is nowhere to be found, but looks like
this might have been missed in a rename. Changing to `value`.

Fixes ansible-collections#172

* integration test
* changelog
Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* Fix UnboundLocalError in sqs_queue
The variable `existing_value` is nowhere to be found, but looks like
this might have been missed in a rename. Changing to `value`.

Fixes ansible-collections#172

* integration test
* changelog
Co-authored-by: Mark Chappell <[email protected]>
alinabuzachis pushed a commit to alinabuzachis/community.aws that referenced this pull request Jul 19, 2021
* Fix UnboundLocalError in sqs_queue
The variable `existing_value` is nowhere to be found, but looks like
this might have been missed in a rename. Changing to `value`.

Fixes ansible-collections#172

* integration test
* changelog
Co-authored-by: Mark Chappell <[email protected]>
danielcotton pushed a commit to danielcotton/community.aws that referenced this pull request Nov 23, 2021
* Fix UnboundLocalError in sqs_queue
The variable `existing_value` is nowhere to be found, but looks like
this might have been missed in a rename. Changing to `value`.

Fixes ansible-collections#172

* integration test
* changelog
Co-authored-by: Mark Chappell <[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 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.

UnboundLocalError in sqs_queue
5 participants