You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by GitBox <gi...@apache.org> on 2022/11/24 03:53:20 UTC
[GitHub] [pinot] snleee commented on a diff in pull request #9855: ADLS file system upgrade
snleee commented on code in PR #9855:
URL: https://github.com/apache/pinot/pull/9855#discussion_r1031033488
##########
pinot-plugins/pinot-file-system/pinot-adls/src/main/java/org/apache/pinot/plugin/filesystem/ADLSGen2PinotFS.java:
##########
@@ -586,15 +579,8 @@ public boolean touch(URI uri)
@Override
public InputStream open(URI uri)
throws IOException {
- // Use Blob API since read() function from Data Lake Client currently takes "OutputStream" as an input and
- // flush bytes to an output stream. This needs to be piped back into input stream to implement this function.
- // On the other hand, Blob API directly allow you to open the input stream.
- BlobClient blobClient = _blobServiceClient.getBlobContainerClient(_fileSystemClient.getFileSystemName())
- .getBlobClient(AzurePinotFSUtil.convertUriToUrlEncodedAzureStylePath(uri));
-
- 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()
Review Comment:
Thanks for addressing this!
##########
pinot-plugins/pinot-file-system/pinot-adls/src/main/java/org/apache/pinot/plugin/filesystem/ADLSGen2PinotFS.java:
##########
@@ -191,20 +182,22 @@ public void init(PinotConfiguration config) {
clientSecretCredentialBuilder.httpClient(builder.build());
dataLakeServiceClientBuilder.credential(clientSecretCredentialBuilder.build());
- blobServiceClientBuilder.credential(clientSecretCredentialBuilder.build());
+ break;
+ }
+ case NONE: {
+ LOGGER.info("Authenticating using anonymous access");
break;
}
default:
- throw new IllegalStateException("Expecting valid authType. One of (ACCESS_KEY, AZURE_AD, AZURE_AD_WITH_PROXY");
+ throw new IllegalArgumentException(
Review 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)?
##########
pinot-plugins/pinot-file-system/pinot-adls/src/main/java/org/apache/pinot/plugin/filesystem/ADLSGen2PinotFS.java:
##########
@@ -191,20 +182,22 @@ public void init(PinotConfiguration config) {
clientSecretCredentialBuilder.httpClient(builder.build());
dataLakeServiceClientBuilder.credential(clientSecretCredentialBuilder.build());
- blobServiceClientBuilder.credential(clientSecretCredentialBuilder.build());
+ break;
+ }
+ case NONE: {
Review Comment:
`ANONYMOUS_ACCESS` may be a better naming for authentication type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org