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
