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

[athena] fix TypeError while delete messages from SQS #598

Merged
merged 1 commit into from
Feb 14, 2018

Conversation

chunyong-lin
Copy link
Contributor

@chunyong-lin chunyong-lin commented Feb 14, 2018

to: @jacknagz or @ryandeivert
cc: @airbnb/streamalert-maintainers
size: small
resolves #590

Background

We noticed there is Athena error due to TypeError while deleting messages from SQS. The root cause of TypeError is because of complicated list comprehension logic when first deletion failed. The list comprehension added extra list around pushed back messages.

Changes

  • Break down complicated list comprehension to simpler ones.
  • Add mock class for SQS client and add unit test case to reproduce error.
  • Add a max_retries in backoff logic to make the code testable.

Testing

  • Rule testing
python manage.py lambda test --processor all
...
StreamAlertCLI [INFO]: (60/60) Successful Tests
StreamAlertCLI [INFO]: (93/93) Alert Tests Passed
StreamAlertCLI [INFO]: Completed
  • Unit testing
./tests/scripts/unit_tests.sh
...
Ran 530 tests in 8.466s

OK

@coveralls
Copy link

coveralls commented Feb 14, 2018

Coverage Status

Coverage increased (+0.07%) to 95.469% when pulling ecab817 on fix_sqs_delete_type_error into 954b70b on master.

Copy link
Contributor

@jacknagz jacknagz left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for the very informative PR body too!

@chunyong-lin chunyong-lin force-pushed the fix_sqs_delete_type_error branch from a2017a2 to ecab817 Compare February 14, 2018 19:55
@jacknagz jacknagz merged commit d508896 into master Feb 14, 2018
@jacknagz jacknagz deleted the fix_sqs_delete_type_error branch February 14, 2018 20:04
@ryandeivert ryandeivert added this to the 1.6.1 milestone Mar 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: SQS Delete TypeError
4 participants