Warn and skip on glacier objects that may fail for s3 commands #1581
Conversation
| @@ -324,4 +326,4 @@ def _list_single_object(self, s3_path): | |||
| file_size = int(response['ContentLength']) | |||
| last_update = parse(response['LastModified']) | |||
| last_update = last_update.astimezone(tzlocal()) | |||
| return s3_path, file_size, last_update | |||
There was a problem hiding this comment.
This seems like a weird API. We're returning parts of the response (such as LastModified) as well as just the entire response whole sale. This can probably be cleaned up.
There was a problem hiding this comment.
I kept it like this to keep parity with the output for local file listings. There is no response associated with a local listing. I will probably break up the logic such that the listing of files and listing of s3 objects do not have the same interface.
kyleknap
added
incorporating-feedback
and removed
pr:needs-review
kyleknap
force-pushed
the
ignore-glacier
branch
3 times, most recently
from
7014821 to
20a8a07
Compare
|
Alright, I think I incorporated the feedback. Refactored the |
kyleknap
added
pr:needs-review
Before we would try to process an operation (FileInfo) no matter what it was if it reached the S3Handler. Now we do some checks to see if the operation will fail given we know the FileInfo object represents a Glacier objects. If it is we know skip it as opposed to letting the error handling handle it which is much slower if you have large/many Glacier objects.
This argument will still cause glacier objects to be skipped over but the skip warning will no longer be printed to stderr for glacier warnings and not affect the return code.
The list_files and list_objects no longer yield the same data. The data required from s3 was no longer the same as the data required from a local file system. The data yielded back from both methods was minimized such that a minimized number of variables needed to be yielded.
Specifically consolidated the local file data into an object when yielding from ``list_files``. It parallels to how the s3 yields it response data.
kyleknap
force-pushed
the
ignore-glacier
branch
from
20a8a07 to
a2dd875
Compare
|
Alright I cleaned up the code a bit more. Should be good to look at. |
|
|
| 'help_text': ( | ||
| 'Turns off glacier warnings. Warnings about operations that cannot ' | ||
| 'be performed because it involves copying, downloading, or moving ' | ||
| 'a glacier object will no longer be printed to standard error and ' |
There was a problem hiding this comment.
Changing this to "and will no longer cause the return code of the command to be 2" might make it a bit more clear that using this option will prevent an error exit rather than cause one.
|
🚢 |
Warn and skip on glacier objects that may fail for s3 commands
* fix: add version to `samconfig.toml` file - support version key, any float is okay. - if a config file is present and the version key is missing, we do not process it. - if a config file is missing, thats fine. this check does not get in the way. - validation logic to determine if a SAM CLI version is compatible can be written later. * bugfix: do not continously read everytime a samconfig.put is called
This pull request does the following:
2. The most important part of this functionality is that we will not even try performing an operation, which could slow down a command especially if you have a lot of or large glacier objects.--ignore-glacier-warningsargument that allows you to hide the glacier warnings and not cause the return code to be 2 if a glacier object is encountered. Note that glacier objects are still always skipped if the command involves downloading, copying, or moving an object when the source is a glacier object.All of the integration tests pass.
Fixes #748
cc @jamesls @mtdowling @rayluo @JordonPhillips