Add unit tests for publish2cloud.py #162
Conversation
boolean5
force-pushed
the
add-tests-for-p2c
branch
from
af55d55
to
6daca9e
Compare
|
I added tests for Note: One of the changes was returning |
|
Just added unit tests for |
Return the checksum of the contents of the list file starting right after the chunk header. Previously, we were always starting from the 26th byte, regardless of the size of the header.
Move the code that uses the configuration file to determine if a list should be uploaded to S3 outside of `new_data_to_publish_to_s3` and into `publish_to_cloud`. Rename `check_upload_remote_settings_config` to `check_upload_config` and adjust it to work both for S3 and remote settings. In `publish_to_cloud` make sure that `new_data_to_publish_to_s3` is only called when `upload_to_s3` is True, to avoid the cost of unnecessarily accessing S3. In addition, when the key corresponding to a list does not yet exist on S3, return True right away instead of creating a key with dummy list contents and comparing against its checksum. Also, fix a minor bug in getting the name of the S3 key from the configuration file. Previously, if the section did not have the `s3_key` option, `new_data_to_publish_to_s3` would raise a `NoOptionError` instead of falling back to using the `output` option as the S3 key name. Raise a ValueError when the `s3_key` option exists but is empty. Finally, add a docstring to describe the functionality of `new_data_to_publish_to_s3`.
Add moto as a requirement and implement unit tests for new_data_to_publish_to_s3.
Add unit tests to check that the expected S3 key permissions are set in new_data_to_publish_to_s3 and publish_to_s3. These tests are currently expected to fail because setting key permissions with add_user_grant() does not work in moto.
boolean5
force-pushed
the
add-tests-for-p2c
branch
from
5d40f71
to
028068b
Compare
| eoh = header.find('\n') | ||
| chunktype, chunknum, hash_size, data_len = header[:eoh].split(':') | ||
| header = fp.readline().rstrip('\n') | ||
| chunktype, chunknum, hash_size, data_len = header.split(':') | ||
| return dict( | ||
| type=chunktype, num=chunknum, hash_size=hash_size, len=data_len, | ||
| checksum=hashlib.sha256(fp.read()).hexdigest() |
There was a problem hiding this comment.
So with this update we will now be getting a more accurate checksum that excludes the header but includes the checksum of the rest of the file. When I merge this to Stage I expect most, if not all, lists to be updated. When this is confirmed I should talk to the ops team to let them know there may be large requests coming in.
There was a problem hiding this comment.
To make use of the updated list better, so that scaling our service is done to deliver real updates and not changes to our list creation script, we may want to coordinate the changes from shavar-prod-lists along with shipping this change to prod.
Implement unit tests for
chunk_metadata,new_data_to_publish_to_s3andpublish_to_s3.Will close #130.