dlmarion commented on code in PR #2986:
URL: https://github.com/apache/accumulo/pull/2986#discussion_r984534728


##########
core/src/main/java/org/apache/accumulo/core/client/admin/InstanceOperations.java:
##########
@@ -93,6 +93,15 @@ void modifyProperties(Consumer<Map<String,String>> 
mapMutator) throws AccumuloEx
    */
   Map<String,String> getSystemConfiguration() throws AccumuloException, 
AccumuloSecurityException;
 
+  /**
+   * Retrieves the stored System properties from zookeeper. This method is 
different from
+   * {@link #getSystemConfiguration()} as it will only return the stored 
properties in zookeeper and
+   * not properties from accumulo.properties or default values..
+   *
+   * @return a map of stored system properties set in zookeeper
+   */
+  Map<String,String> getStoredProperties() throws AccumuloException, 
AccumuloSecurityException;

Review Comment:
   So, I looked at NamespaceOperations and TableOperations. Both of them have a 
`getProperties` method that returns properties scoped at the appropriate level. 
On InstanceOperations we have:
   
   getSystemConfiguration() -> returns the entire running configuration
   getSiteConfiguration() -> returns the properties set in `accumulo.properties`
   getSystemProperties() -> we're proposing to resurrect this method because of 
a failed test, which would return only the properties stored in ZooKeeper.
   
   Why would a user, via the AccumuloClient, need to know where the properties 
are stored? Why should they care? I can see this is a server-side capability, 
but why does it exist in the client API? I would propose the following:
   
     1. that we modify InstanceOperations to have a `getProperties` method that 
returns properties at the instance level just like NamespaceOperations and 
TableOperations.
     2. That we deprecate (and remove) getSiteConfiguration and 
getSystemProperties in InstanceOperations. There is no reason that 
PropStoreConfigIT can't use a server-side API to get the information that it 
needs for the test.
   
   



-- 
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]

Reply via email to