Fix SNOW-683245 and format the file #1188
Conversation
| @@ -296,7 +296,8 @@ public void download( | |||
| } | |||
|
|
|||
| logger.debug("Starting download without presigned URL", false); | |||
| blob.downloadTo(localFile.toPath()); | |||
| blob.downloadTo( | |||
There was a problem hiding this comment.
Is there test coverage for this? Or are there any existing tests that this would affect?
There was a problem hiding this comment.
We change the way we access GCS. So existing tests will do.
There was a problem hiding this comment.
So this is not a behavior change? Does JDBC do the decompression somewhere else previously? @sfc-gh-igarish
There was a problem hiding this comment.
No behavior change. In the same method below it does decompression. So download file as is and then decompress locally.Same logic as pre-signed url.
|
Can we add a test to ensure that the driver is successfully able to unzip the file to protect against future regressions? |
Do we have a test for GCS with pre-signed URL and GET? If yes, then it will cover. Up to now we won't get this error because this feature was not enable in GS. So I think, it's not a regression. |
|
| @@ -18,13 +18,13 @@ public void testParseAccountName() throws SnowflakeSQLException { | |||
| cstring.getParameters().get(SFSessionProperty.ACCOUNT.getPropertyKey().toUpperCase()), | |||
| is("abc")); | |||
|
|
|||
| // Hostname should remain unchanged by default. | |||
There was a problem hiding this comment.
Is this a new bug in our test?
There was a problem hiding this comment.
This is not part of my PR. Today again when I tried to do git push, if failed. So I did "git pull origin my_branch" and it got other commits in this PR. Before doing pull, I tried "git rebase" but still git push was failing.
| * | ||
| * @throws Throwable | ||
| */ | ||
| @Test | ||
| @Ignore | ||
| @ConditionalIgnoreRule.ConditionalIgnore(condition = RunningOnGithubAction.class) |
There was a problem hiding this comment.
Just to double check that this is what we want. run it on Jenkins but not on github
There was a problem hiding this comment.
Yes. All GCS tests are configure that way.
afe997f to
8a00b7f
Compare
|
SonarQube Quality Gate
|
Overview
SNOW-683245: Bug fix in JDBC for GET command when GCS_USE_DOWNSCOPED_CREDENTIAL in GS is true
When code was added not to use presigned url and use GCS API to download the file, it was using GCS GUNZIP stuff. In pre-signed URL logic the driver download the file as is i.e. in GZIP format. So added same logic when use the GCS API to get file without decompression i.e. in RAW format of the file. This optimize the data transfer on the wire.
Sent the new JDBC Jar to Timothy as he has setup already. He confirm the change works fine.
External contributors - please answer these questions before submitting a pull request. Thanks!
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 #NNNN
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Please write a short description of how your code change solves the related issue.
Pre-review checklist