Github user spmallette commented on a diff in the pull request:

    https://github.com/apache/tinkerpop/pull/583#discussion_r108741558
  
    --- Diff: 
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Settings.java 
---
    @@ -387,6 +387,18 @@ public SerializerSettings() {}
             public String className = AllowAllAuthenticator.class.getName();
     
             /**
    +         * Enable audit logging of authenticated users and gremlin 
evaluation requests.
    +         */
    +        public boolean enableAuditLog = false;
    +
    +        /**
    +         * The fully qualified class name of the {@link 
AuthenticationHandler} implementation.
    +         * This class name will be used to load the implementation from 
the classpath.
    +         * Defaults to null when not specified.
    +         */
    +        public String handlerClassName = null;
    --- End diff --
    
    I tried to stay consistent in naming for classes in the config by going 
with `className` where the key was representative of an object in a `Map` or 
`List`, but now it kinda bites us as `authentication.className` is used for the 
`Authenticator` when it now really should probably refer to the handler since 
it is now configurable. 
    
    Here's an idea:
    
    1. Rename `handlerClassName` to `authenticationHandler`
    2. Overload `className` with `authenticator` - if `authenticator` is 
present then ignore any value in `className`. Update all the yaml files to use 
`authenticator`. 
[Deprecate](http://tinkerpop.apache.org/docs/current/dev/developer/#_deprecation)
 `AuthenticationSettings.className`.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to