hooks: lambda: allow uploading pre-built payloads #564
Conversation
| includes, | ||
| excludes, | ||
| follow_symlinks) | ||
| return _upload_prebuilt_zip(s3_conn, bucket, prefix, name, options, |
There was a problem hiding this comment.
Kinda bugs me that we have to methods that call _upload_code. I think _upload_prebuilt_zip and _build_and_upload_zip should be changed so that we return a tuple (zip_file, version) and then we call _upload_code once.
There was a problem hiding this comment.
I've kept them separated due to the prebuilt case passing the open file through instead of loading all the contents. When that happens _upload_code needs to be called in the scope of the context manager, which will require some weird conditionals if the call is unified.
There was a problem hiding this comment.
Gotcha. _build_and_upload_zip currently doesn't use the context manager when passing the zip file down to _upload_code. Think we should change it so we're consistent?
| @@ -93,6 +93,18 @@ def _calculate_hash(files, root): | |||
| return file_hash.hexdigest() | |||
|
|
|||
|
|
|||
| def _calculate_prebuilt_hash(f): | |||
There was a problem hiding this comment.
I think we can just re-use _calculate_hash.
_calculate_hash("path/to/zip-file.zip")There was a problem hiding this comment.
That won't be the exact MD5 of the file, as it will append a null character to the stream when calculating.
There was a problem hiding this comment.
Is that a problem? We just need a consistent hash.
There was a problem hiding this comment.
Probably not, but it seemed more natural to me (as I just used md5sum to figure out the reference values).
1323b63 to
6824154
Compare
|
@ejholmes @danielkza - thinking about making a 1.4 release soon. Should this be in it? What's left to figure out? |
6824154 to
1917033
Compare
|
@phobologic Rebased and refactored a bit to reduce code duplication. |
1917033 to
e359e10
Compare
e359e10 to
359619a
Compare
|
Rebased, including a refactor of the Lambda hook tests. |
|
Fixed the broken tests. |
No description provided.