pareshddevalia commented on code in PR #730:
URL: https://github.com/apache/atlas/pull/730#discussion_r3893539576
##########
notification/src/main/java/org/apache/atlas/notification/rest/RestNotification.java:
##########
@@ -115,11 +114,10 @@ private AtlasClientV2 setupAtlasClientV2(Configuration
configuration) throws Atl
String[] atlasEndPoint =
configuration.getStringArray(ATLAS_HOOK_REST_NOTIFICATION_ENDPOINT);
if (isEndpointNotSpecified(atlasEndPoint)) {
- atlasEndPoint =
configuration.getStringArray(AtlasConstants.ATLAS_REST_ADDRESS_KEY);
- }
-
- if (isEndpointNotSpecified(atlasEndPoint)) {
- atlasEndPoint = new String[] {DEFAULT_ATLAS_URL};
+ throw new AtlasException("atlas.hook.rest.notification.address
must be configured when REST notification is enabled. "
Review Comment:
can we update this throw new
AtlasException(ATLAS_HOOK_REST_NOTIFICATION_ENDPOINT + " is required for
rest-notification-webapp (e.g. "
+ AtlasConstants.DEFAULT_REST_NOTIFICATION_ADDRESS +
")");
##########
notification/src/test/java/org/apache/atlas/notification/RestNotificationTest.java:
##########
@@ -150,36 +152,48 @@ public void
testRestNotificationEndpointPrefersHookRestAddress() throws Exceptio
assertEquals(configuredEndpoints[0],
"http://atlas-rest.example.com:41000/rest");
}
- @Test
- public void testRestNotificationEndpointFallsBackToAtlasRestAddress()
throws Exception {
+ @Test(expectedExceptions = AtlasException.class)
+ public void testRestNotificationFailsFastWhenHookAddressNotConfigured()
throws Exception {
Configuration localConf = new BaseConfiguration();
- localConf.setProperty("atlas.rest.address",
"http://atlas-main.example.com:21000");
localConf.setProperty("atlas.rest.basic.auth.username", "admin");
localConf.setProperty("atlas.rest.basic.auth.password", "admin123");
- RestNotification restNotification = new RestNotification(localConf);
-
- String[] configuredEndpoints =
getConfiguredBaseUrls(restNotification.atlasClientV2);
-
- assertEquals(configuredEndpoints.length, 1);
- assertEquals(configuredEndpoints[0],
"http://atlas-main.example.com:21000");
+ new RestNotification(localConf);
}
- @Test
- public void
testRestNotificationFallsBackToAtlasRestWhenNotificationAddressesAllBlank()
throws Exception {
+ @Test(expectedExceptions = AtlasException.class)
+ public void testRestNotificationFailsFastWhenHookAddressesAllBlank()
throws Exception {
Configuration localConf = new BaseConfiguration();
localConf.addProperty("atlas.hook.rest.notification.address", "");
localConf.addProperty("atlas.hook.rest.notification.address", " ");
localConf.setProperty("atlas.rest.address",
"http://atlas-main.example.com:21000");
localConf.setProperty("atlas.rest.basic.auth.username", "admin");
localConf.setProperty("atlas.rest.basic.auth.password", "admin123");
- RestNotification restNotification = new RestNotification(localConf);
+ new RestNotification(localConf);
+ }
- String[] configuredEndpoints =
getConfiguredBaseUrls(restNotification.atlasClientV2);
+ @Test(expectedExceptions = AtlasException.class)
+ public void
testRestNotificationFailsFastWhenOnlyAtlasRestAddressConfigured() throws
Exception {
+ Configuration localConf = new BaseConfiguration();
+ localConf.setProperty("atlas.rest.address",
"http://atlas-main.example.com:21000");
+ localConf.setProperty("atlas.rest.basic.auth.username", "admin");
+ localConf.setProperty("atlas.rest.basic.auth.password", "admin123");
- assertEquals(configuredEndpoints.length, 1);
- assertEquals(configuredEndpoints[0],
"http://atlas-main.example.com:21000");
+ new RestNotification(localConf);
+ }
+
+ @Test
+ public void testRestNotificationFailFastExceptionMessage() {
+ Configuration localConf = new BaseConfiguration();
+
+ try {
+ new RestNotification(localConf);
+ fail("Expected AtlasException when
atlas.hook.rest.notification.address is not configured");
+ } catch (AtlasException e) {
+
assertTrue(e.getMessage().contains("atlas.hook.rest.notification.address must
be configured"));
Review Comment:
instead of atlas.hook.rest.notification.address must be configured you can
add atlas.hook.rest.notification.address is required
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]