Github user revans2 commented on the pull request: https://github.com/apache/storm/pull/1147#issuecomment-191986179 I am going through StatsUtil.java and it is more of the same thing everywhere. The code is a literal translation of the clojure code. The input APIs in most cases are clojure specific, but many of these functions actually return thrift object already. For example `stats/agg-comp-execs-stats` returns an instance of `ComponentPageInfo`. I would prefer to see us have a shim that will translate the clojure maps, etc into java objects that would be a better java API/state when we translate the clojure code that is calling into it. For example ``` (defn agg-comp-execs-stats "Aggregate various executor statistics for a component from the given heartbeats." [exec->host+port task->component beats window include-sys? topology-id topology component-id] ``` was translated into ``` public static TopologyPageInfo aggTopoExecsStats( String topologyId, Map exec2nodePort, Map task2component, Map beats, StormTopology topology, String window, boolean includeSys, IStormClusterState clusterState); ``` but if we expand the signature a bit more it really looks like ``` public static TopologyPageInfo aggTopoExecsStats( String topologyId, Map<List<Number>/*low exec, high exec*/,List<Object>/*String host, Number port*/ > exec2nodePort, Map<Integer, String> task2component, Map beats<List<Number>/*low exec, high exec*/, Map<Keyword, Object>/*Clojure map keys :stats, :uptime, :time-secs, good luck*/ >, StormTopology topology, String window, boolean includeSys, IStormClusterState clusterState); ``` If we want a more java like API with classes instead of maps/Lists it should probably be something more like ``` public class HostPort { public final String host; public final int port; ... }; public static TopologyPageInfo aggTopoExecsStats( String topologyId, Map<ExecutorInfo, HostPort> exec2nodePort, Map<Integer, String> task2component, Map beats<ExecutorInfo, ExecutorBeat>, StormTopology topology, String window, boolean includeSys, IStormClusterState clusterState); ``` All of the public facing APIs need this type of a translation to happen. If you want me to go through all of them and list what I think they should be I can.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---