-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revamp AWS file support to add support for multipart uploads in glacier testing #257
base: master
Are you sure you want to change the base?
Conversation
…2_with_mfs_revamp_extended_WIP
…dd changelog. Bump minor version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good and should help the testing generally, just a few small comments
@@ -47,6 +52,13 @@ | |||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not clear what "cloning" means here?
# 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
return result | ||
|
||
def restore_object(self, Bucket, Key, RestoreRequest, VersionId: Optional[str] = None, | ||
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small typo: Documentation
# ==================== | ||
scenario(1) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these not be split into separate tests?
In new module
bucket_utils.py
:parse_s3_object_name
In
common.py
:New glacier-related constants:
STANDARD
REDUCED_REDUNDANCY
STANDARD_IA
ONEZONE_IA
INTELLIGENT_TIERING
GLACIER
DEEP_ARCHIVE
OUTPOSTS
GLACIER_IR
New type hint
S3ObjectNameSpec
In
glacier_utils.py
:Allow a
version_id=
argument toGlacierUtils.is_restore_finished
Some improved error messages.
Some small code refactors.
In
misc_utils.py
:make_counter
threadsafe so that threaded functionality can call it.In
qa_utils.py
:Support for mock glacier testing in
MockBotoS3Client
for methods:create_multipart_upload
upload_part_copy
complete_multipart_upload
Revamp the abstractions for managing MockFileSystem to allow for centralized changes that might be needed to handle new file content types, such as
MockAbstractContent
MockBigContent
for mocking large files quickly and space-efficiently.MockPartableBytes
for mocking small content that still wants to test piecewise-copying in support of the multipart upload protocol.