Revamp AWS file support to add support for multipart uploads in glacier testing #257
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…2_with_mfs_revamp_extended_WIP
…dd changelog. Bump minor version.
willronchetti
approved these changes
May 26, 2023
| exported(QA_EXCEPTION_PATTERN, find_uses, confirm_no_uses, VersionChecker, ChangeLogChecker) | ||
|
|
||
|
|
||
| def make_unique_token(monotonic=False): # effectively a guid but for things that don't promise specifically a guid |
There was a problem hiding this comment.
Might be useful to include info on what the intended use of this is for?
| pass | ||
|
|
||
|
|
||
| class MockPartableContent(MockAbstractContent): |
There was a problem hiding this comment.
Docstring should note partable == simulating AWS S3 multi part upload
| class MockPartableContent(MockAbstractContent): | ||
|
|
||
| @classmethod | ||
| def start_cloning_from(cls, content): |
There was a problem hiding this comment.
Not clear what "cloning" means here?
Comment on lines
+625
to
+626
| # We might at some future point want to consider whether callers of this function should | ||
| # see auto-mirroring or versioning. |
There was a problem hiding this comment.
These types of comments seem to be a mix of docstring vs. inline comments, may be worth generating docstrings for all of these and adding the notes where appropriate
| StorageClass: Optional[S3StorageClass] = None): | ||
| duration_days: int = RestoreRequest.get('Days') | ||
| duration_days = RestoreRequest.get('Days') | ||
| # NOTE: Dcoumentation says "Days element is required for regular restores, and must not be provided |
There was a problem hiding this comment.
Small typo: Documentation
Comment on lines
+2290
to
+2292
| # ==================== | ||
| scenario(1) | ||
|
|
There was a problem hiding this comment.
Should these not be split into separate tests?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In new module
bucket_utils.py:parse_s3_object_nameIn
common.py:New glacier-related constants:
STANDARDREDUCED_REDUNDANCYSTANDARD_IAONEZONE_IAINTELLIGENT_TIERINGGLACIERDEEP_ARCHIVEOUTPOSTSGLACIER_IRNew type hint
S3ObjectNameSpecIn
glacier_utils.py:Allow a
version_id=argument toGlacierUtils.is_restore_finishedSome improved error messages.
Some small code refactors.
In
misc_utils.py:make_counterthreadsafe so that threaded functionality can call it.In
qa_utils.py:Support for mock glacier testing in
MockBotoS3Clientfor methods:create_multipart_uploadupload_part_copycomplete_multipart_uploadRevamp the abstractions for managing MockFileSystem to allow for centralized changes that might be needed to handle new file content types, such as
MockAbstractContentMockBigContentfor mocking large files quickly and space-efficiently.MockPartableBytesfor mocking small content that still wants to test piecewise-copying in support of the multipart upload protocol.