nickwallen commented on a change in pull request #1458: METRON-2177 Upgrade
Profiler for HBase 2.0.2
URL: https://github.com/apache/metron/pull/1458#discussion_r311098015
##########
File path:
metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/VerboseProfile.java
##########
@@ -122,35 +129,28 @@ public Object apply(List<Object> args, Context context)
throws ParseException {
groups = getArg(3, List.class, args);
}
- // get globals from the context
- Map<String, Object> globals = (Map<String, Object>)
context.getCapability(GLOBAL_CONFIG)
- .orElse(Collections.emptyMap());
-
- // lazily create the profiler client, if needed
Review comment:
The `PROFILE_VERBOSE` function has lazy initialization, but will not respond
to changes in the global config like `PROFILE_GET` does. This is how I
initially implemented `PROFILE_VERBOSE` in #1292 (right or wrong).
I believe the thought process was `PROFILE_VERBOSE` is not a "production"
function and is only useful for troubleshooting in the REPL. So if you change
the global config, you're going to have to restart the REPL. But to your point,
why have different behaviors for these very similar functions and violate
[POLA](https://en.wikipedia.org/wiki/Principle_of_least_astonishment)?
How about for this PR, I ...
(1) Add comments describing the disparity.
(2) Create a JIRA to alter `PROFILE_VERBOSE` to behave like `PROFILE_GET`
and respond to changes in the global configuration.
I think it is a worthwhile improvement, I'm just not sure how much code
will need to change to make this happen on this already large PR.
----------------------------------------------------------------
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
