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:
[email protected]


Reply via email to