rajarshisarkar commented on code in PR #4489:
URL: https://github.com/apache/iceberg/pull/4489#discussion_r843549805
##########
dell/src/main/java/org/apache/iceberg/dell/ecs/EcsFileIO.java:
##########
@@ -74,6 +82,18 @@ public void initialize(Map<String, String> properties) {
this.dellProperties = new DellProperties(properties);
this.dellClientFactory = DellClientFactories.from(properties);
this.s3 = dellClientFactory::ecsS3;
+
+ // Report Hadoop metrics if Hadoop is available
+ try {
+ DynConstructors.Ctor<MetricsContext> ctor =
+
DynConstructors.builder(MetricsContext.class).hiddenImpl(DEFAULT_METRICS_IMPL,
String.class).buildChecked();
+ this.metrics = ctor.newInstance("ecs");
+
+ metrics.initialize(properties);
+ } catch (NoClassDefFoundError | NoSuchMethodException | ClassCastException
e) {
+ this.metrics = MetricsContext.nullMetrics();
Review Comment:
Actually that is being overridden in the current logic. I have added some
in-line comments below to highlight the issue:
```
private MetricsContext metrics = MetricsContext.nullMetrics(); // metrics
initialised as nullMetrics at class level
...
...
@Override
public void initialize(Map<String, String> properties) {
...
...
try {
DynConstructors.Ctor<MetricsContext> ctor =
DynConstructors.builder(MetricsContext.class).hiddenImpl(DEFAULT_METRICS_IMPL,
String.class).buildChecked();
this.metrics = ctor.newInstance("ecs"); // metrics re-initialised as
new HadoopMetricsContext("ecs")
metrics.initialize(properties); // throws exception in case of
classpath issues
} catch (NoClassDefFoundError | NoSuchMethodException |
ClassCastException e) {
// We swallow the exception and allow new HadoopMetricsContext() to
flow down to EcsInputFile/EcsOutputFile
LOG.warn("Unable to load metrics class: '{}', falling back to null
metrics", DEFAULT_METRICS_IMPL, e);
}
}
```
We can handle the fallback in the catch block (do suggest if there is some
better way). Without that the CI seems to fail for commit:
[c7e355f](https://github.com/apache/iceberg/pull/4489/commits/c7e355fc0820c5630218ac0e5ae1f125852e210a)
The CI passes if we handle the nullMetrics fallback in the catch block for
commit:
[2ad9cda](https://github.com/apache/iceberg/pull/4489/commits/2ad9cdae82ed3714ee58f96abfc95daf1b880a58)
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
