[ 
https://issues.apache.org/jira/browse/GOBBLIN-1222?focusedWorklogId=469884&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-469884
 ]

ASF GitHub Bot logged work on GOBBLIN-1222:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 12/Aug/20 19:02
            Start Date: 12/Aug/20 19:02
    Worklog Time Spent: 10m 
      Work Description: autumnust commented on a change in pull request #3070:
URL: https://github.com/apache/incubator-gobblin/pull/3070#discussion_r469473372



##########
File path: 
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveDatasetFinder.java
##########
@@ -107,6 +107,7 @@
   private static final String FAILURE_CONTEXT = "FailureContext";
 
   protected final Properties properties;
+  public Properties getProperties() { return properties; }

Review comment:
       lombok annotation ? 

##########
File path: 
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java
##########
@@ -139,17 +141,28 @@ public FileAwareInputStreamDataWriter(State state, int 
numBranches, int branchId
     URI uri = URI.create(uriStr);
     this.fs = FileSystem.get(uri, conf);
     this.fileContext = FileContext.getFileContext(uri, conf);
+    this.copyableDatasetMetadata =
+        
CopyableDatasetMetadata.deserialize(state.getProp(CopySource.SERIALIZED_COPYABLE_DATASET));
 
+    /**
+     * The staging directory defines the path of staging folder.
+     * USER_DEFINED_STATIC_STAGING_DIR_FLAG shall be set to true when user 
wants to specify the staging folder and the directory can be fetched through 
USER_DEFINED_STATIC_STAGING_DIR property.
+     * IS_DATASET_STAGING_DIR_USED when true creates the staging folder within 
a dataset location for dataset copying.
+     * Else system will calculate the staging directory automatically.
+     */
     if 
(state.getPropAsBoolean(ConfigurationKeys.USER_DEFINED_STAGING_DIR_FLAG,false)) 
{
       this.stagingDir = new 
Path(state.getProp(ConfigurationKeys.USER_DEFINED_STATIC_STAGING_DIR));
+    } else if 
((state.getPropAsBoolean(ConfigurationKeys.IS_DATASET_STAGING_DIR_USED,false))) 
{
+      String stg_dir = state.getProp(DATASET_STAGING_DIR_PATH) + 
STAGING_DIR_SUFFIX + "/" + state.getProp(ConfigurationKeys.JOB_NAME_KEY ) + "/" 
+ state.getProp(ConfigurationKeys.JOB_ID_KEY);

Review comment:
       Naming of variable to be consistent. check 
https://gobblin.readthedocs.io/en/latest/developer-guide/CodingStyle/

##########
File path: 
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java
##########
@@ -88,6 +88,8 @@
   public static final boolean DEFAULT_GOBBLIN_COPY_CHECK_FILESIZE = false;
   public static final String GOBBLIN_COPY_TASK_OVERWRITE_ON_COMMIT = 
"gobblin.copy.task.overwrite.on.commit";
   public static final boolean DEFAULT_GOBBLIN_COPY_TASK_OVERWRITE_ON_COMMIT = 
false;
+  public static final String STAGING_DIR_SUFFIX = "/taskstaging";

Review comment:
       double check if the original one has a dash in the between. 

##########
File path: 
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopySource.java
##########
@@ -32,6 +32,8 @@
 import lombok.AllArgsConstructor;
 import lombok.extern.slf4j.Slf4j;
 
+import org.apache.gobblin.data.management.copy.hive.HiveDataset;

Review comment:
       the import ordering needs to consistent with the style file. 

##########
File path: 
gobblin-utility/src/main/java/org/apache/gobblin/util/WriterUtils.java
##########
@@ -92,6 +92,17 @@ public static Path getWriterStagingDir(State state, int 
numBranches, int branchI
         WriterUtils.getWriterFilePath(state, numBranches, branchId));
   }
 
+  public static Path getDatasetWriterStagingDir(State state, int numBranches, 
int branchId) {

Review comment:
       You would like to reduce the code duplication here and re-use the method 
that is already implemented above. 

##########
File path: 
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/hive/HiveDataset.java
##########
@@ -330,4 +338,13 @@ private boolean canCopyTable() {
     }
     return true;
   }
+
+  public String getLastSegment(Path location) {

Review comment:
       Path provides API to get this last segment. 




----------------------------------------------------------------
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:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 469884)
    Time Spent: 1.5h  (was: 1h 20m)

> Create right abstraction to assemble dataset staging dir for Hive dataset 
> finder
> --------------------------------------------------------------------------------
>
>                 Key: GOBBLIN-1222
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-1222
>             Project: Apache Gobblin
>          Issue Type: Bug
>            Reporter: Vaibhav Arya
>            Priority: Major
>          Time Spent: 1.5h
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to