Add support for Azure Blob #641
Conversation
There was a problem hiding this comment.
Minor comments only.
Can you change the commit message of e913b4a to Update the version of hadoop-apache2.
Can you squash 415b1f7 and db37ed8? The feature and its unit test should be in a single commit. Also can you re-phrase the commit message to Add configuration variables for authenticating to Azure Blob.
| @@ -93,6 +95,8 @@ public HdfsConfigurationUpdater(HiveClientConfig hiveClientConfig, HiveS3Config | |||
| this.fileSystemMaxCacheSize = hiveClientConfig.getFileSystemMaxCacheSize(); | |||
|
|
|||
| this.s3AwsAccessKey = s3Config.getS3AwsAccessKey(); | |||
| this.wasbAccessKey = wasbConfig.getWasbAccessKey(); | |||
There was a problem hiding this comment.
You're cutting the S3 'paragraph' into two by placing the two lines here. I suggest starting another code 'paragraph' just for blob either after or before the S3 'paragraph'.
|
|
||
| // set config for Azure Blob | ||
| if ((wasbAccessKey != null) && (wasbStorageAccount != null)) { | ||
| config.set("fs.azure.account.key." + wasbStorageAccount + ".blob.core.windows.net", wasbAccessKey); |
| @@ -208,6 +212,11 @@ public void updateConfiguration(PrestoHadoopConfiguration config) | |||
| config.setLong(PrestoS3FileSystem.S3_MULTIPART_MIN_PART_SIZE, s3MultipartMinPartSize.toBytes()); | |||
| config.setBoolean(PrestoS3FileSystem.S3_PIN_CLIENT_TO_CURRENT_REGION, pinS3ClientToCurrentRegion); | |||
| config.set(PrestoS3FileSystem.S3_USER_AGENT_PREFIX, s3UserAgentPrefix); | |||
|
|
|||
| // set config for Azure Blob | |||
| if ((wasbAccessKey != null) && (wasbStorageAccount != null)) { | |||
There was a problem hiding this comment.
The parentheses around each comparison are unnecessary. Equality comparisons have higher precedence that logical AND.
| @@ -91,7 +91,8 @@ public static DistributedQueryRunner createQueryRunner(Iterable<TpchTable<?>> ta | |||
|
|
|||
| HiveClientConfig hiveClientConfig = new HiveClientConfig(); | |||
| HiveS3Config s3Config = new HiveS3Config(); | |||
| HdfsConfiguration hdfsConfiguration = new HiveHdfsConfiguration(new HdfsConfigurationUpdater(hiveClientConfig, s3Config)); | |||
| HiveWasbConfig wasbConfig = new HiveWasbConfig(); | |||
There was a problem hiding this comment.
No need to create a separate variable, just inline it where it is used like you do elsewhere.
| public class TestHiveWasbConfig | ||
| { | ||
| @Test | ||
| public void testExplicitPropertyMappings() |
No description provided.