autumnust commented on a change in pull request #3070: URL: https://github.com/apache/incubator-gobblin/pull/3070#discussion_r461093604
########## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopySource.java ########## @@ -195,6 +197,32 @@ datasetFinder instanceof IterableDatasetFinder ? (IterableDatasetFinder<CopyableDatasetBase>) datasetFinder : new IterableDatasetFinderImpl<>(datasetFinder); + if (state.getPropAsBoolean(ConfigurationKeys.DATASET_STAGING_DIR,false)){ Review comment: Some comments here: - This code block has too much duplication with the following block. - what's trying to achieve here? The implementation details like "hiveDatasetFinder" should not be leaking in the upstream constructs like `CopySource` ########## File path: gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveDataset.java ########## @@ -125,6 +126,8 @@ public HiveDataset(FileSystem fs, HiveMetastoreClientPool clientPool, Table tabl Optional.fromNullable(this.table.getDataLocation()); this.tableIdentifier = this.table.getDbName() + "." + this.table.getTableName(); + this.datasetStagingDir = properties.getProperty("hive.dataset.copy.target.table.prefixToBeReplaced") + "/" + this.table.getDbName() + "/" + this.table.getTableName(); Review comment: Please avoid using string value directly but find its corresponding variable which should have been defined in the codebase. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org