Fix auto-compressed dataset downloads #194
Merged
Conversation
This file contains 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
Notes and other fixes: - Previously, we needed to unzip the file in the unit test because it was actually fetching the archive. This is because our `DatasetHttpResolver` doesn't put the filename in the path (it passes it in a query string). Rather than change the implementation in the resolver, I moved filename handling to the shared stubbed route. - Added mocking for downloads to get 302 redirected (as it does in prod to GCS). This was needed to get `response.url` to be accurate in `clients.py`, which drives the auto-decompression feature. - Added other files to support the auto-(de)compression logic. - In the interest of keeping this PR small, a follow up PR will add tests for the Models and Competitions HTTP tests. That may necessitate moving some routes/etc. around to be shared beyond Datasets. Fixes #187 http://b/379756505
goeffthomas
added a commit
that referenced
this pull request
Dec 16, 2024
Notes: - This is follow on work from #194 - Also added some integration tests - Note that not all stubs have been moved to this redirect path. Only the paths that could have compressed URLs in prod have been affected (Comps and Datasets). We can move all download paths to use this if we'd like. http://b/379756505
goeffthomas
added a commit
that referenced
this pull request
Dec 16, 2024
Notes: - This is follow on work from #194 - Also added some integration tests - Note that not all stubs have been moved to this redirect path. Only the paths that could have compressed URLs in prod have been affected (Comps and Datasets). We can move all download paths to use this if we'd like. http://b/379756505
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Notes and other fixes:
DatasetHttpResolverdoesn't put the filename in the path (it passes it in a query string). Rather than change the implementation in the resolver, I moved filename handling to the shared stubbed route. cc @jeward414response.urlto be accurate inclients.py, which drives the auto-decompression feature.Fixes #187
http://b/379756505