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

ASF GitHub Bot logged work on BEAM-6382:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 09/Jan/19 01:24
            Start Date: 09/Jan/19 01:24
    Worklog Time Spent: 10m 
      Work Description: xinyuiscool commented on pull request #7443: 
[BEAM-6382] SamzaRunner: add an option to read configs using a user-defined 
factory
URL: https://github.com/apache/beam/pull/7443#discussion_r246229183
 
 

 ##########
 File path: 
runners/samza/src/main/java/org/apache/beam/runners/samza/translation/ConfigBuilder.java
 ##########
 @@ -73,30 +74,28 @@ public Config build() {
       config.put(
           "beamPipelineOptions",
           Base64Serializer.serializeUnchecked(new 
SerializablePipelineOptions(options)));
-      // TODO: remove after SAMZA-1531 is resolved
-      config.put(
-          ApplicationConfig.APP_RUN_ID,
-          String.valueOf(System.currentTimeMillis())
-              + "-"
-              // use the most significant bits in UUID (8 digits) to avoid 
collision
-              + UUID.randomUUID().toString().substring(0, 8));
 
       return new MapConfig(config);
     } catch (Exception e) {
       throw new RuntimeException(e);
     }
   }
 
-  private static Map<String, String> createUserConfig(SamzaPipelineOptions 
options) {
+  private static Map<String, String> createUserConfig(SamzaPipelineOptions 
options) throws Exception {
     final String configFilePath = options.getConfigFilePath();
     final Map<String, String> config = new HashMap<>();
 
     // If user provides a config file, use it as base configs.
     if (StringUtils.isNoneEmpty(configFilePath)) {
       final File configFile = new File(configFilePath);
-      checkArgument(configFile.exists(), "Config file %s does not exist", 
configFilePath);
-      final PropertiesConfigFactory configFactory = new 
PropertiesConfigFactory();
       final URI configUri = configFile.toURI();
+      final ConfigFactory configFactory = 
options.getConfigFactory().getDeclaredConstructor().newInstance();
+
+      // Config file must exist for default properties config
+      if (configFactory instanceof PropertiesConfigFactory) {
+        checkArgument(configFile.exists(), "Config file %s does not exist", 
configFilePath);
 
 Review comment:
   Currently in Yarn the configFilePath won't be valid in the container, since 
we load the configs from the am url. But the command line args are still passed 
through. In the future we might be able to fix this logic. I will mark a TODO 
here.
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


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

    Worklog Id:     (was: 182792)
    Time Spent: 50m  (was: 40m)

> SamzaRunner: add an option to read configs using a user-defined factory
> -----------------------------------------------------------------------
>
>                 Key: BEAM-6382
>                 URL: https://issues.apache.org/jira/browse/BEAM-6382
>             Project: Beam
>          Issue Type: Improvement
>          Components: runner-samza
>            Reporter: Xinyu Liu
>            Assignee: Xinyu Liu
>            Priority: Major
>          Time Spent: 50m
>  Remaining Estimate: 0h
>
> We need an option to read configs from a factory which is useful in Yarn as 
> well as user-defined file format. By default this config factory is to read 
> property file.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to