[
https://issues.apache.org/jira/browse/BEAM-6382?focusedWorklogId=182778&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-182778
]
ASF GitHub Bot logged work on BEAM-6382:
----------------------------------------
Author: ASF GitHub Bot
Created on: 09/Jan/19 00:40
Start Date: 09/Jan/19 00:40
Worklog Time Spent: 10m
Work Description: lhaiesp 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_r246218495
##########
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:
why do we only check this for PropertiesConfigFactory? If configFilePath is
not empty, shouldn't it always be a valid path anyway?
----------------------------------------------------------------
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: 182778)
Time Spent: 0.5h (was: 20m)
> 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: 0.5h
> 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)
