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

Fix #3760 Avoid downloading migrations from S3 unless they will actually be run, when possible #3764

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

OneGeek
Copy link

@OneGeek OneGeek commented Oct 19, 2023

Addresses #3760

… from S3 rather than directly computing it to avoid downloading the content before it is necessary.

Addresses flyway#3760
@OneGeek
Copy link
Author

OneGeek commented Oct 19, 2023

As currently implemented this retrieves the crc32 checksum that can be stored in an S3 object upon upload. If it wasn't supplied, or fails to retrieve the checksum for any reason, it falls back to the current behavior of downloading the file and computing the checksum locally.

Note that in order for S3 to have the CRC32 available, it needs to have been included (or marked for computation) at the time of the upload to S3, otherwise it'll be null. S3 does have an etag field that's a hash of the contents which could be grabbed and then checksummed with crc32 in order to make things work without needing to do anything in particular when uploading to S3, but then the migration checksum when stored on S3 would differ from the same migration stored locally. Depending on exactly the goals of the flyway project, that (or an option to choose which behavior) may or may not be acceptable, but I don't have enough context to make that call, so I chose the conservative approach.

I expect to need to update docs before the PR is accepted, but wanted to confirm this change is acceptable before putting in that effort.

@OneGeek
Copy link
Author

OneGeek commented Nov 30, 2023

Is this something that might be incorporated into flyway in the near future?

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.

None yet

1 participant