Skip to content

Conversation

@austinbyers
Copy link
Collaborator

@austinbyers austinbyers commented Sep 12, 2017

A collection of minor code cleanups before releasing v1.0 of BinaryAlert!

Bug Fixes

  • terraform init is required before any other terraform operation (fixes deploys; thanks @fusionrace for flagging)
  • Unit tests no longer do a real pip install of downloader dependencies

Code Changes

Terraform Changes

  • Adds force_destroy variable to optionally allow complete terraform destroy of S3 buckets
  • Doubles dispatch limit (from 50 to 100) to better accommodate batch operations

Test Changes

Tested

python3 manage.py configure  # no downloader
python3 manage.py deploy
python3 manage.py live_test
python3 manage.py configure # downloader
python3 manage.py clone_rules
python3 manage.py deploy
python3 manage.py live_test
# Manual test of downloader

Reviewers

to: @ryandeivert
cc: @airbnb/binaryalert-maintainers

@austinbyers austinbyers added this to the v1.0.0 milestone Sep 12, 2017
@coveralls
Copy link

Coverage Status

Coverage increased (+6.06%) to 95.52% when pulling 4595c5f on abb--cleanup into 510f94a on master.

Copy link
Contributor

@ryandeivert ryandeivert left a comment

Choose a reason for hiding this comment

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

Nice work on this!

# In order to prevent spurious alerts about new S3 objects, we report S3 objects from
# the previous Lambda version as well.
previous_s3_objects = {}
previous_s3_objects: Set[str] = set()
Copy link
Contributor

Choose a reason for hiding this comment

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

super interesting syntax for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I actually didn't realize for awhile that you could annotate variables. But that explains why method arguments are annotated the same way (with default values), e.g.

def func(path: Optional[str] = None)

@austinbyers austinbyers merged commit 54b0589 into master Sep 12, 2017
@austinbyers austinbyers deleted the abb--cleanup branch September 12, 2017 21:01
@coveralls
Copy link

Coverage Status

Coverage increased (+6.06%) to 95.52% when pulling 58730be on abb--cleanup into 4298b99 on master.

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.

Mock out loggers during unit tests Use higher-level boto3 resources Use type annotations

4 participants