Skip to content
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

[feat] add optional versioning id for s3 archive #4

Merged
merged 1 commit into from
Sep 7, 2024

Conversation

Ryang20718
Copy link
Contributor

@Ryang20718 Ryang20718 commented Aug 29, 2024

This PR adds file versioning for s3_archive in order to solve the problem defined below

Consider the following problem:

if person A updates the object in the s3 bucket, it will break person's B code since s3 archive now points at the new object. with this versioning enabled, we could update the object, but not break person B's code

The file_version field can be added to s3_archive if the s3 bucket is versioned

    s3_archive(
		<kwargs>
        file_version = "SI56h6qfu3O6AYdy5soRntQvajXqRXbe3",
    )

resolves #3

Testing:

  • Regression testing s3 archives without file version
  • Regression testing s3 archives with bad sha256
  • Regression testing s3 archives with file version
  • Regression testing s3 archives with file version bad 256
  • Regression testing s3 archives with file version that doesn't exist

Sample error message if version id is invalid

An error occurred (InvalidArgument) when calling the GetObject operation: Invalid version id specified

file_arg = ["--key", file_path]
file_version_arg = ["--version-id", file_version] if file_version else []
src_url = filename
cmd = [tool_path] + extra_flags + ["s3api", "get-object"] + bucket_arg + file_arg + file_version_arg + [filename]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aws s3 doesn't support versioning. That's why I had to shift to s3api

@Ryang20718
Copy link
Contributor Author

@1e100 when you have a moment, would appreciate a review :)

Copy link
Owner

@1e100 1e100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks straightforward enough, LGTM

@1e100 1e100 merged commit 1f556ec into 1e100:master Sep 7, 2024
@Ryang20718
Copy link
Contributor Author

thanks! when you have a moment, if you could cut a new release, would be much appreciated!

@Ryang20718 Ryang20718 deleted the user/ryang/s3_versioning branch September 8, 2024 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Versioning for object items
2 participants