sv2000 commented on a change in pull request #3113:
URL: https://github.com/apache/incubator-gobblin/pull/3113#discussion_r500463929
##########
File path:
gobblin-utility/src/main/java/org/apache/gobblin/util/PullFileLoader.java
##########
@@ -284,43 +287,62 @@ private Config findAndLoadGlobalConfigInDirectory(Path
path, Config fallback) th
* @return The {@link Config} in path with fallback as fallback.
* @throws IOException
*/
- private Config loadJavaPropsWithFallback(Path propertiesPath, Config
fallback) throws IOException {
+ private Config loadJavaPropsWithFallback(Path propertiesPath, Config
fallback)
+ throws IOException {
PropertiesConfiguration propertiesConfiguration = new
PropertiesConfiguration();
- try (InputStreamReader inputStreamReader = new
InputStreamReader(this.fs.open(propertiesPath),
- Charsets.UTF_8)) {
-
propertiesConfiguration.setDelimiterParsingDisabled(ConfigUtils.getBoolean(fallback,
- PROPERTY_DELIMITER_PARSING_ENABLED_KEY,
DEFAULT_PROPERTY_DELIMITER_PARSING_ENABLED_KEY));
+ try (InputStreamReader inputStreamReader = new
InputStreamReader(this.fs.open(propertiesPath), Charsets.UTF_8)) {
+ propertiesConfiguration.setDelimiterParsingDisabled(ConfigUtils
+ .getBoolean(fallback, PROPERTY_DELIMITER_PARSING_ENABLED_KEY,
DEFAULT_PROPERTY_DELIMITER_PARSING_ENABLED_KEY));
propertiesConfiguration.load(inputStreamReader);
Config configFromProps =
ConfigUtils.propertiesToConfig(ConfigurationConverter.getProperties(propertiesConfiguration));
return
ConfigFactory.parseMap(ImmutableMap.of(ConfigurationKeys.JOB_CONFIG_FILE_PATH_KEY,
-
PathUtils.getPathWithoutSchemeAndAuthority(propertiesPath).toString()))
- .withFallback(configFromProps)
+
PathUtils.getPathWithoutSchemeAndAuthority(propertiesPath).toString())).withFallback(configFromProps)
.withFallback(fallback);
} catch (ConfigurationException ce) {
+ log.error("Failed to load Java properties from file at {} due to {}",
propertiesPath, ce.getLocalizedMessage());
throw new IOException(ce);
}
}
- private Config loadHoconConfigAtPath(Path path) throws IOException {
- try (InputStream is = fs.open(path);
- Reader reader = new InputStreamReader(is, Charsets.UTF_8)) {
- return
ConfigFactory.parseMap(ImmutableMap.of(ConfigurationKeys.JOB_CONFIG_FILE_PATH_KEY,
- PathUtils.getPathWithoutSchemeAndAuthority(path).toString()))
- .withFallback(ConfigFactory.parseReader(reader,
ConfigParseOptions.defaults().setSyntax(ConfigSyntax.CONF)));
+ private Config loadHoconConfigAtPath(Path path)
+ throws IOException {
+ try (InputStream is = fs.open(path); Reader reader = new
InputStreamReader(is, Charsets.UTF_8)) {
+ return ConfigFactory.parseMap(ImmutableMap
+ .of(ConfigurationKeys.JOB_CONFIG_FILE_PATH_KEY,
PathUtils.getPathWithoutSchemeAndAuthority(path).toString()))
+ .withFallback(ConfigFactory.parseReader(reader,
ConfigParseOptions.defaults().setSyntax(ConfigSyntax.CONF)));
+ } catch (ConfigException configException) {
+ throw wrapConfigException(path, configException);
}
}
- private Config loadHoconConfigWithFallback(Path path, Config fallback)
throws IOException {
- try (InputStream is = fs.open(path);
- Reader reader = new InputStreamReader(is, Charsets.UTF_8)) {
- return
ConfigFactory.parseMap(ImmutableMap.of(ConfigurationKeys.JOB_CONFIG_FILE_PATH_KEY,
- PathUtils.getPathWithoutSchemeAndAuthority(path).toString()))
- .withFallback(ConfigFactory.parseReader(reader,
ConfigParseOptions.defaults().setSyntax(ConfigSyntax.CONF)))
- .withFallback(fallback);
+ /**
+ * Wrap a {@link ConfigException} (which extends {@link RuntimeException}
with an IOException,
+ * with a helpful message if possible
+ * @param path
+ * @param configException
+ * @return an {@link IOException} wrapping the passed in ConfigException.
+ */
+ private IOException wrapConfigException(Path path, ConfigException
configException) {
+ if (configException.origin() != null) {
+ return new IOException("Failed to parse config file " + path.toString()
+ + " at lineNo:" +
configException.origin().lineNumber(),configException);
Review comment:
Nit: Add a whitespace after the ",".
##########
File path:
gobblin-utility/src/main/java/org/apache/gobblin/util/PullFileLoader.java
##########
@@ -175,14 +176,14 @@ public Config apply(@Nullable Path jobFile) {
}
try {
- return PullFileLoader.this.loadPullFile(jobFile,
- sysProps, loadGlobalProperties);
- } catch (IOException e) {
- log.error("Cannot load job from {} due to {}", jobFile,
ExceptionUtils.getFullStackTrace(e));
+ return PullFileLoader.this.loadPullFile(jobFile, sysProps,
loadGlobalProperties);
+ } catch (IOException ie) {
+ log.error("",ie);
Review comment:
Add white space after ",".
##########
File path:
gobblin-utility/src/main/java/org/apache/gobblin/util/PullFileLoader.java
##########
@@ -175,14 +176,14 @@ public Config apply(@Nullable Path jobFile) {
}
try {
- return PullFileLoader.this.loadPullFile(jobFile,
- sysProps, loadGlobalProperties);
- } catch (IOException e) {
- log.error("Cannot load job from {} due to {}", jobFile,
ExceptionUtils.getFullStackTrace(e));
+ return PullFileLoader.this.loadPullFile(jobFile, sysProps,
loadGlobalProperties);
+ } catch (IOException ie) {
+ log.error("",ie);
return null;
}
}
- });
+ }).stream().filter(x -> x!= null).collect(Collectors.toList());
Review comment:
Add whitespaces around "!=".
##########
File path:
gobblin-utility/src/main/java/org/apache/gobblin/util/PullFileLoader.java
##########
@@ -175,14 +176,14 @@ public Config apply(@Nullable Path jobFile) {
}
try {
- return PullFileLoader.this.loadPullFile(jobFile,
- sysProps, loadGlobalProperties);
- } catch (IOException e) {
- log.error("Cannot load job from {} due to {}", jobFile,
ExceptionUtils.getFullStackTrace(e));
+ return PullFileLoader.this.loadPullFile(jobFile, sysProps,
loadGlobalProperties);
+ } catch (IOException ie) {
+ log.error("",ie);
return null;
}
}
- });
+ }).stream().filter(x -> x!= null).collect(Collectors.toList());
+ // only return valid parsed configs
Review comment:
Can this behavior be documented in the javadoc of the method?
----------------------------------------------------------------
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]
