You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by "chaitalicod (via GitHub)" <gi...@apache.org> on 2025/11/24 06:40:23 UTC

[PR] ATLAS-5162 : Add support to load custom application properties [atlas]

chaitalicod opened a new pull request, #476:
URL: https://github.com/apache/atlas/pull/476

   ## What changes were proposed in this pull request?
   
   (Please fill in changes proposed in this fix. Create an issue in ASF JIRA before opening a pull request and
   set the title of the pull request which starts with
   the corresponding JIRA issue number. (e.g. ATLAS-XXXX: Fix a typo in YYY))
   
   
   ## How was this patch tested?
   
   (Please explain how this patch was tested. Ex: unit tests, manual tests)
   (If this patch involves UI changes, please attach a screenshot; otherwise, remove this)


-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "pinal-shah (via GitHub)" <gi...@apache.org>.
pinal-shah commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2579632210


##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,36 +124,63 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String absoluteFilePath) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(absoluteFilePath));
+                }
+            }
+        }
+        return instance;
+    }
+
+    public static Configuration getConf(PropertiesConfiguration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration get(String fileName) throws AtlasException {
         try {
-            URL url;
+            URL url = null;

Review Comment:
   null assignment is not required



##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,36 +124,63 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String absoluteFilePath) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(absoluteFilePath));
+                }
+            }
+        }
+        return instance;
+    }
+
+    public static Configuration getConf(PropertiesConfiguration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration get(String fileName) throws AtlasException {
         try {
-            URL url;
+            URL url = null;
 
-            if (confLocation == null) {
-                LOG.info("Looking for {} in classpath", fileName);
+            if (StringUtils.isNotBlank(fileName)) {
+                File file = new File(fileName);
+                if (file.exists()) {
+                    url = file.toURI().toURL();
+                    LOG.info("Loading {} from {}", fileName, url);
+                }
+            }
+            if (url == null) {
+                String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
 
-                url = ApplicationProperties.class.getClassLoader().getResource(fileName);
+                if (confLocation == null) {
+                    LOG.info("Looking for {} in classpath", fileName);
 
-                if (url == null) {
-                    LOG.info("Looking for /{} in classpath", fileName);
+                    url = ApplicationProperties.class.getClassLoader().getResource(fileName);
 
-                    url = ApplicationProperties.class.getClassLoader().getResource("/" + fileName);
+                    if (url == null) {
+                        LOG.info("Looking for /{} in classpath", fileName);
+                        url = ApplicationProperties.class.getClassLoader().getResource("/" + fileName);
+                    }
+                } else {
+                    url = new File(confLocation, fileName).toURI().toURL();
                 }
-            } else {
-                url = new File(confLocation, fileName).toURI().toURL();
-            }
 
-            LOG.info("Loading {} from {}", fileName, url);
+                LOG.info("Loading {} from {}", fileName, url);
+            }
 
             ApplicationProperties appProperties = new ApplicationProperties(url);
-

Review Comment:
   please retain this



-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "chaitalicod (via GitHub)" <gi...@apache.org>.
chaitalicod merged PR #476:
URL: https://github.com/apache/atlas/pull/476


-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "chaitalicod (via GitHub)" <gi...@apache.org>.
chaitalicod commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2580167343


##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,27 +124,57 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String absoluteFilePath) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(absoluteFilePath));
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration getConf(PropertiesConfiguration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
+
+    public static Configuration get(String fileName) throws AtlasException {
         try {
-            URL url;
+            URL url = null;
 
-            if (confLocation == null) {
-                LOG.info("Looking for {} in classpath", fileName);
+            if (StringUtils.isNotBlank(fileName)) {
+                File file = new File(fileName);
+                if (file.exists()) {
+                    url = file.toURI().toURL();
+                    LOG.info("Loading {} from {}", fileName, url);

Review Comment:
   No this log will not be printed if removed for this usecase of custom path because the later one is for conf dir/classpath 



-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "pinal-shah (via GitHub)" <gi...@apache.org>.
pinal-shah commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2575689209


##########
intg/pom.xml:
##########
@@ -115,6 +115,12 @@
             </exclusions>
         </dependency>
 
+        <dependency>

Review Comment:
   avoid using extra dependency if not needed



##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -132,12 +140,12 @@ public void testGetFileAsInputStream() throws Exception {
 
     @Test
     public void verifySetDefault() throws AtlasException {
-        Configuration         props  = ApplicationProperties.get("test.properties");
+        Configuration props = ApplicationProperties.get("test.properties");

Review Comment:
   retain original formatting of this code



##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
 
     @Test
     public void verifyGetLatesttString() throws AtlasException {
-        String        key       = "atlas.metadata.namespace";
-        String        oldVal    = "nm-sp-1";
-        String        newVal    = "nm-sp-2";
+        String key = "atlas.metadata.namespace";

Review Comment:
   retain original formatting



##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -19,20 +19,28 @@
 
 import org.apache.atlas.utils.AtlasConfigurationUtil;
 import org.apache.commons.configuration.Configuration;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
 import org.testng.annotations.Test;
 
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.AbstractMap;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotEquals;
 import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertSame;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
 import static org.testng.Assert.fail;
 
 /**
  * Unit test for {@link ApplicationProperties}
  *
  */
+

Review Comment:
   nit



##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
 
     @Test
     public void verifyGetLatesttString() throws AtlasException {
-        String        key       = "atlas.metadata.namespace";
-        String        oldVal    = "nm-sp-1";
-        String        newVal    = "nm-sp-2";
+        String key = "atlas.metadata.namespace";
+        String oldVal = "nm-sp-1";
+        String newVal = "nm-sp-2";
         Configuration atlasConf = ApplicationProperties.get("test.properties");
 
         assertEquals(atlasConf.getString(key), oldVal);
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, key, oldVal), newVal);
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, "garbage", oldVal), oldVal);
     }
+
+    @Test
+    public void verifyCustomisedPath() throws AtlasException, IOException {
+        Configuration atlasConf = ApplicationProperties.getConf("src/test/resources/test.properties");
+        InputStream inStr = null;
+
+        try {
+            inStr = ApplicationProperties.getFileAsInputStream(atlasConf, "jaas.properties.file", null);
+            assertNotNull(inStr);
+        } finally {
+            if (inStr != null) {
+                inStr.close();
+            }
+        }
+    }
+
+    @Test
+    public void verifyPropertyValues() throws AtlasException {
+        Configuration props = ApplicationProperties.getConf("src/test/resources/test.properties");
+        ApplicationProperties aProps = (ApplicationProperties) props;
+
+        String defaultValue = "atlas";
+        String someKey = "atlas.service";
+
+        assertFalse(aProps.getString(someKey).equals(defaultValue));

Review Comment:
   can you add true value as well



##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
 
     @Test
     public void verifyGetLatesttString() throws AtlasException {
-        String        key       = "atlas.metadata.namespace";
-        String        oldVal    = "nm-sp-1";
-        String        newVal    = "nm-sp-2";
+        String key = "atlas.metadata.namespace";
+        String oldVal = "nm-sp-1";
+        String newVal = "nm-sp-2";
         Configuration atlasConf = ApplicationProperties.get("test.properties");
 
         assertEquals(atlasConf.getString(key), oldVal);
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, key, oldVal), newVal);
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, "garbage", oldVal), oldVal);
     }
+
+    @Test
+    public void verifyCustomisedPath() throws AtlasException, IOException {
+        Configuration atlasConf = ApplicationProperties.getConf("src/test/resources/test.properties");
+        InputStream inStr = null;
+
+        try {
+            inStr = ApplicationProperties.getFileAsInputStream(atlasConf, "jaas.properties.file", null);

Review Comment:
   please review if this is required



##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -148,13 +156,73 @@ public void verifySetDefault() throws AtlasException {
 
     @Test
     public void verifyGetLatesttString() throws AtlasException {
-        String        key       = "atlas.metadata.namespace";
-        String        oldVal    = "nm-sp-1";
-        String        newVal    = "nm-sp-2";
+        String key = "atlas.metadata.namespace";
+        String oldVal = "nm-sp-1";
+        String newVal = "nm-sp-2";
         Configuration atlasConf = ApplicationProperties.get("test.properties");
 
         assertEquals(atlasConf.getString(key), oldVal);
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, key, oldVal), newVal);
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, "garbage", oldVal), oldVal);
     }
+
+    @Test
+    public void verifyCustomisedPath() throws AtlasException, IOException {
+        Configuration atlasConf = ApplicationProperties.getConf("src/test/resources/test.properties");
+        InputStream inStr = null;
+
+        try {
+            inStr = ApplicationProperties.getFileAsInputStream(atlasConf, "jaas.properties.file", null);
+            assertNotNull(inStr);
+        } finally {
+            if (inStr != null) {
+                inStr.close();
+            }
+        }
+    }
+
+    @Test
+    public void verifyPropertyValues() throws AtlasException {
+        Configuration props = ApplicationProperties.getConf("src/test/resources/test.properties");
+        ApplicationProperties aProps = (ApplicationProperties) props;
+
+        String defaultValue = "atlas";
+        String someKey = "atlas.service";
+
+        assertFalse(aProps.getString(someKey).equals(defaultValue));
+    }
+
+    @Test
+    public void verifyCustomisedPathFailureExpected() {
+        try (MockedStatic<ApplicationProperties> mocked = Mockito.mockStatic(ApplicationProperties.class)) {

Review Comment:
   not required, if you find issues, you can use ApplicationProperties.forceReload()



##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,36 +124,68 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String propertyFileName) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(propertyFileName));
+                }
+            }
+        }
+        return instance;
+    }
 
-        try {
-            URL url;
+    public static Configuration getConf(Configuration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
 
-            if (confLocation == null) {
-                LOG.info("Looking for {} in classpath", fileName);
+    public static Configuration get(String fileName) throws AtlasException {
+        try {
+            URL url = null;
+
+            if (StringUtils.isNotBlank(fileName)) {
+                File file = new File(fileName);
+                boolean hasPath = file.getParent() != null;
+                if (hasPath) {

Review Comment:
   Why is this check needed?



-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "jayendrap (via GitHub)" <gi...@apache.org>.
jayendrap commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2580011568


##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,27 +124,57 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String absoluteFilePath) throws AtlasException {

Review Comment:
   Consider renaming to method name to "getConfFromAbsolutePath"



##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,27 +124,57 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String absoluteFilePath) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(absoluteFilePath));
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration getConf(PropertiesConfiguration clientConf) throws AtlasException {

Review Comment:
   Consider renaming the method name to "get" instead of "getConf"



##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,27 +124,57 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String absoluteFilePath) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(absoluteFilePath));
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration getConf(PropertiesConfiguration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
+
+    public static Configuration get(String fileName) throws AtlasException {
         try {
-            URL url;
+            URL url = null;
 
-            if (confLocation == null) {
-                LOG.info("Looking for {} in classpath", fileName);
+            if (StringUtils.isNotBlank(fileName)) {
+                File file = new File(fileName);
+                if (file.exists()) {
+                    url = file.toURI().toURL();
+                    LOG.info("Loading {} from {}", fileName, url);

Review Comment:
   Same log is also printed at the end of the method, so we can remove this.



-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "pinal-shah (via GitHub)" <gi...@apache.org>.
pinal-shah commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2559994555


##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,36 +126,67 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String propertyFileName) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(propertyFileName));
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration getConf(Configuration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
+
+    public static Configuration get(String fileName) throws AtlasException {
         try {
-            URL url;
+            URL url = null;
+
+            Path path = null;
+            path = Paths.get(fileName);
 
-            if (confLocation == null) {
-                LOG.info("Looking for {} in classpath", fileName);
+            if (path != null && path.getParent() != null) {

Review Comment:
   path is always != null, hence should be removed



-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "pinal-shah (via GitHub)" <gi...@apache.org>.
pinal-shah commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2559992157


##########
intg/src/test/java/org/apache/atlas/ApplicationPropertiesTest.java:
##########
@@ -157,4 +157,16 @@ public void verifyGetLatesttString() throws AtlasException {
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, key, oldVal), newVal);
         assertEquals(AtlasConfigurationUtil.getRecentString(atlasConf, "garbage", oldVal), oldVal);
     }
+
+    @Test
+    public void verifyCustomisedPath() throws AtlasException {

Review Comment:
   please add testcases of all the usecase



-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "pinal-shah (via GitHub)" <gi...@apache.org>.
pinal-shah commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2560016208


##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,36 +126,67 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String propertyFileName) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(propertyFileName));
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration getConf(Configuration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
+
+    public static Configuration get(String fileName) throws AtlasException {
         try {
-            URL url;
+            URL url = null;
+
+            Path path = null;
+            path = Paths.get(fileName);
 
-            if (confLocation == null) {
-                LOG.info("Looking for {} in classpath", fileName);
+            if (path != null && path.getParent() != null) {
+                // Case A: User passed a full path
+                String confLocation = path.getParent().toString();
+                String propertyFileName = path.getFileName().toString();
 
-                url = ApplicationProperties.class.getClassLoader().getResource(fileName);
+                url = new File(confLocation, propertyFileName).toURI().toURL();
+                LOG.info("Loading {} from {}", fileName, url);

Review Comment:
   please review if line number 153 to 165 can be replaced by
   ```
   if (StringUtils.isNotEmpty(filepath) {
      File file = new File(filepath);
      if (file.exists()) {
         url = file.toURI().toURL();
      }
      
      if (url == null) {
   ```



-- 
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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] ATLAS-5162 : Add support to load custom application properties [atlas]

Posted by "pinal-shah (via GitHub)" <gi...@apache.org>.
pinal-shah commented on code in PR #476:
URL: https://github.com/apache/atlas/pull/476#discussion_r2579632210


##########
intg/src/main/java/org/apache/atlas/ApplicationProperties.java:
##########
@@ -124,36 +124,63 @@ public static Configuration set(Configuration configuration) throws AtlasExcepti
         return instance;
     }
 
-    public static Configuration get(String fileName) throws AtlasException {
-        String confLocation = System.getProperty(ATLAS_CONFIGURATION_DIRECTORY_PROPERTY);
+    public static Configuration getConf(String absoluteFilePath) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(get(absoluteFilePath));
+                }
+            }
+        }
+        return instance;
+    }
+
+    public static Configuration getConf(PropertiesConfiguration clientConf) throws AtlasException {
+        if (instance == null) {
+            synchronized (ApplicationProperties.class) {
+                if (instance == null) {
+                    set(clientConf);
+                }
+            }
+        }
+        return instance;
+    }
 
+    public static Configuration get(String fileName) throws AtlasException {
         try {
-            URL url;
+            URL url = null;

Review Comment:
   null assignment is not 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: dev-unsubscribe@atlas.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org