You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@trafficcontrol.apache.org by GitBox <gi...@apache.org> on 2020/08/03 14:57:32 UTC
[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #4857: fixed TR to use default watcher URL when parameter is not present
zrhoffman commented on a change in pull request #4857:
URL: https://github.com/apache/trafficcontrol/pull/4857#discussion_r464466162
##########
File path: traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/AbstractResourceWatcher.java
##########
@@ -57,7 +57,7 @@ public void configure(final JsonNode config) {
}
final WatcherConfig watcherConfig = new WatcherConfig(getWatcherConfigPrefix(), config, trafficOpsUtils);
- final String resourceUrl = (watcherConfig.getUrl() != null && !watcherConfig.getUrl().isEmpty()) ? watcherConfig.getUrl() : getDataBaseURL();
+ final String resourceUrl = (watcherConfig.getUrl() != null && !watcherConfig.getUrl().isEmpty()) ? watcherConfig.getUrl() : getDefaultDatabaseURL();
Review comment:
We set `dataBaseURL` later in this function but use the old value of `dataBaseURL between here and there. For those cases, would it make sense to use the condition in this ternary expression to conditionally set `dataBaseURL` to `defaultDatabaseURL` right before this line?
##########
File path: traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/loc/AbstractServiceUpdater.java
##########
@@ -67,6 +68,15 @@ public String getDataBaseURL() {
return dataBaseURL;
}
+ /**
+ * Gets the defaultDatabaseURL.
+ *
+ * @return the defaultDatabaseURL
+ */
+ public String getDefaultDatabaseURL() {
+ return defaultDatabaseURL;
+ }
+
Review comment:
No need for an accessor since only this class and subclasses use defaultDatabaseURL.
##########
File path: traffic_router/core/src/main/java/com/comcast/cdn/traffic_control/traffic_router/core/util/AbstractResourceWatcher.java
##########
@@ -57,7 +57,7 @@ public void configure(final JsonNode config) {
}
final WatcherConfig watcherConfig = new WatcherConfig(getWatcherConfigPrefix(), config, trafficOpsUtils);
- final String resourceUrl = (watcherConfig.getUrl() != null && !watcherConfig.getUrl().isEmpty()) ? watcherConfig.getUrl() : getDataBaseURL();
+ final String resourceUrl = (watcherConfig.getUrl() != null && !watcherConfig.getUrl().isEmpty()) ? watcherConfig.getUrl() : getDefaultDatabaseURL();
Review comment:
This can be `defaultDatabaseURL` instead of `getDefaultDatabaseURL()`.
----------------------------------------------------------------
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:
users@infra.apache.org