SNOW-761256 Fix finish_download in gcs_storage_client #1488
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
Description After super().finish_download(), self.intermediate_dst_path has either been deleted or removed. We should use the dst file name instead. Testing Existing test. I'm curious to learn how to test as well
sfc-gh-sfan
requested review from
a team,
sfc-gh-stan and
sfc-gh-achandrasekaran
and removed request for
a team
|
What's really interesting is that the file is still downloaded and we could return the correct GET result. Given this is running in multithreading and only on a child thread, this stacktrace is actually not brought up. I'm planning to use caplog to make check if there are exceptions, but let me know if there is a better way otherwise. |
Codecov Report
@@ Coverage Diff @@
## main #1488 +/- ##
==========================================
- Coverage 81.95% 81.91% -0.04%
==========================================
Files 61 61
Lines 8616 8616
Branches 1273 1273
==========================================
- Hits 7061 7058 -3
- Misses 1223 1225 +2
- Partials 332 333 +1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
sfc-gh-aalam
approved these changes
Mar 24, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Description
After super().finish_download(), self.intermediate_dst_path has either been deleted or removed. We should use the dst file name instead.
Testing
Existing test. I'm curious to learn how to test as well
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-761256
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
After super().finish_download(), self.intermediate_dst_path has either been deleted or removed. We should use the dst file name instead.