Ethanlm commented on a change in pull request #3306:
URL: https://github.com/apache/storm/pull/3306#discussion_r467680482
##########
File path: storm-client/src/jvm/org/apache/storm/utils/Utils.java
##########
@@ -1188,28 +1188,18 @@ static boolean isValidConf(Map<String, Object> orig,
Map<String, Object> deser)
public static TopologyInfo getTopologyInfo(String name, String asUser,
Map<String, Object> topoConf) {
try (NimbusClient client =
NimbusClient.getConfiguredClientAs(topoConf, asUser)) {
- String topologyId = getTopologyId(name, client.getClient());
- if (null != topologyId) {
- return client.getClient().getTopologyInfo(topologyId);
- }
- return null;
+ return client.getClient().getTopologyInfoByName(name);
} catch (Exception e) {
throw new RuntimeException(e);
}
}
public static String getTopologyId(String name, Nimbus.Iface client) {
try {
- ClusterSummary summary = client.getClusterInfo();
- for (TopologySummary s : summary.get_topologies()) {
- if (s.get_name().equals(name)) {
- return s.get_id();
- }
- }
+ return client.getTopologySummaryByName(name).get_id();
} catch (Exception e) {
throw new RuntimeException(e);
}
- return null;
Review comment:
> I noticed that there is syntax change here. Before the change,
`getTopologyId` returns `null` when it couldn't get the summary. But with the
change, it will throw exceptions when there is no such topology with this
`name`. Is there any impact here?
This change has some impact on backward compatibility since the syntax is
changed here. The caller of this method might reply on the `returned result ==
null` to know if the topology is alive or not.
For example:
https://github.com/apache/storm/blob/master/storm-core/src/jvm/org/apache/storm/command/SetLogLevel.java#L58-L59
User code might do this too. Maybe `Utils. getTopologyId` should return
`null` if the topology is not alive / does not exist.
----------------------------------------------------------------
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:
us...@infra.apache.org
