Conversation
xiangfu0
changed the title
xiangfu0
force-pushed
the
adls-default-credential
branch
from
0ff60b6 to
66bb5e4
Compare
xiangfu0
force-pushed
the
adls-default-credential
branch
from
66bb5e4 to
175bf58
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #9855 +/- ##
============================================
- Coverage 70.34% 70.32% -0.03%
+ Complexity 5014 4936 -78
============================================
Files 1972 1972
Lines 105692 105687 -5
Branches 15993 15988 -5
============================================
- Hits 74350 74321 -29
- Misses 26137 26170 +33
+ Partials 5205 5196 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
...file-system/pinot-adls/src/main/java/org/apache/pinot/plugin/filesystem/ADLSGen2PinotFS.java
Outdated
Show resolved
Hide resolved
| } | ||
| default: | ||
| throw new IllegalStateException("Expecting valid authType. One of (ACCESS_KEY, AZURE_AD, AZURE_AD_WITH_PROXY"); | ||
| throw new IllegalArgumentException( |
There was a problem hiding this comment.
Should we try to initialize the client using DefaultAzureCredential to have a way to pass the credentials to azure client using ENV variables (or system properties)? Or, that is not the scope of this PR?
There was a problem hiding this comment.
Make sense, updated the default behavior to use DefaultAzureCredentialBuilder: https://learn.microsoft.com/en-us/java/api/com.azure.identity.defaultazurecredentialbuilder?view=azure-java-stable
| return blobClient.openInputStream(); | ||
| // Another approach is to download the file to the local disk to a temp path and return the file input stream. In | ||
| // this case, we need to override "close()" and delete temp file. | ||
| return _fileSystemClient.getFileClient(AzurePinotFSUtil.convertUriToUrlEncodedAzureStylePath(uri)).openInputStream() |
There was a problem hiding this comment.
Thanks for addressing this!
|
@jackjlli please also check, there is a behavior change for handling the default scenario for the AzurePinotFS. Since it's on the failover code path, I won't mark it as backward incompatible. |
xiangfu0
added
0.12.0
release-notes
3 participants
NONE, meaning no cred is added.