lhaiesp commented on a change in pull request #992: SAMZA-2163: Add try-catch
to get the version in MetricsSnapshotReporterFactory
URL: https://github.com/apache/samza/pull/992#discussion_r274694452
##########
File path:
samza-core/src/main/scala/org/apache/samza/metrics/reporter/MetricsSnapshotReporterFactory.scala
##########
@@ -45,14 +45,17 @@ class MetricsSnapshotReporterFactory extends
MetricsReporterFactory with Logging
val jobId = config
.getJobId
- val taskClass = Option(new ApplicationConfig(config).getAppClass())
- .getOrElse(config.getTaskClass.getOrElse(throw new SamzaException("No
task or app class defined for config.")))
-
- val version =
Option(Class.forName(taskClass).getPackage.getImplementationVersion)
- .getOrElse({
- warn("Unable to find implementation version in jar's meta info.
Defaulting to 0.0.1.")
- "0.0.1"
- })
+ val version =
+ try {
+ val taskClass = Option(new ApplicationConfig(config).getAppClass())
+ .orElse(config.getTaskClass).get
+
Option(Class.forName(taskClass).getPackage.getImplementationVersion).get
+ } catch {
+ case e: Exception => {
+ warn("Unable to find implementation version in jar's meta info.
Defaulting to 0.0.1.")
Review comment:
Should we log the exception? And we should document why we need to catch
exception here (anonymous class..), so that we don't lose context in the codes.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
