Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-981

Deprecate support for credentialsDbLocation in Gremlin Server Config

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Implemented
    • Affects Version/s: 3.0.2-incubating
    • Fix Version/s: 3.1.1-incubating
    • Component/s: server
    • Labels:
      None

      Description

      The credentialsDbLocation was created to deal with the fact that TinkerGraph did not support persistence. Now that we have gremlin.tinkergraph.graphLocation in the standard TinkerGraph config we can deprecate that old config option and simply rely on the configuration supplied to TinkerGraph itself.

        Activity

        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/incubator-tinkerpop/pull/160

        Show
        githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/incubator-tinkerpop/pull/160
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user pluradj commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/160#issuecomment-160763774

        Ran clean install, followed by integration tests against gremlin-server.
        Ran a couple manual tests with gremlin console and REST.

        VOTE: +1

        Show
        githubbot ASF GitHub Bot added a comment - Github user pluradj commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/160#issuecomment-160763774 Ran clean install, followed by integration tests against gremlin-server. Ran a couple manual tests with gremlin console and REST. VOTE: +1
        Hide
        githubbot ASF GitHub Bot added a comment -

        Github user PommeVerte commented on the pull request:

        https://github.com/apache/incubator-tinkerpop/pull/160#issuecomment-160431972

        This is a nice change to have. Straightforward enough.
        `mvn verify -DskipIntegrationTests=false -pl gremlin-server` passes for me

        VOTE: +1

        Show
        githubbot ASF GitHub Bot added a comment - Github user PommeVerte commented on the pull request: https://github.com/apache/incubator-tinkerpop/pull/160#issuecomment-160431972 This is a nice change to have. Straightforward enough. `mvn verify -DskipIntegrationTests=false -pl gremlin-server` passes for me VOTE: +1
        Hide
        githubbot ASF GitHub Bot added a comment -

        GitHub user spmallette opened a pull request:

        https://github.com/apache/incubator-tinkerpop/pull/160

        TINKERPOP3-981 - Deprecate credentialsDbLocation setting for gremlin server simple auth.

        https://issues.apache.org/jira/browse/TINKERPOP3-981

        Fixed up docs and altered packaged config files. This is a non-breaking change as the setting is still supported. It offers a warning when it is used. Kept tests in place and added new ones to test both features.

        ```text
        mvn clean install
        mvn verify -DskipIntegrationTests=false -pl gremlin-server
        ```

        Did the above tests and some manual testing of REST as well as the driver in the console.

        VOTE: +1

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-981

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/incubator-tinkerpop/pull/160.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #160


        commit 0f4ad252c4097b888ba0df33333d11c512d3af32
        Author: Stephen Mallette <spmva@genoprime.com>
        Date: 2015-11-27T23:25:13Z

        Deprecate credentialsDbLocation setting for gremlin server simple auth.

        Fixed up docs and altered packaged config files. This is a non-breaking change as the setting is still supported. It offers a warning when it is used. Kept tests in place and added new ones to test both features.


        Show
        githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/incubator-tinkerpop/pull/160 TINKERPOP3-981 - Deprecate credentialsDbLocation setting for gremlin server simple auth. https://issues.apache.org/jira/browse/TINKERPOP3-981 Fixed up docs and altered packaged config files. This is a non-breaking change as the setting is still supported. It offers a warning when it is used. Kept tests in place and added new ones to test both features. ```text mvn clean install mvn verify -DskipIntegrationTests=false -pl gremlin-server ``` Did the above tests and some manual testing of REST as well as the driver in the console. VOTE: +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-981 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/incubator-tinkerpop/pull/160.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #160 commit 0f4ad252c4097b888ba0df33333d11c512d3af32 Author: Stephen Mallette <spmva@genoprime.com> Date: 2015-11-27T23:25:13Z Deprecate credentialsDbLocation setting for gremlin server simple auth. Fixed up docs and altered packaged config files. This is a non-breaking change as the setting is still supported. It offers a warning when it is used. Kept tests in place and added new ones to test both features.
        Hide
        spmallette stephen mallette added a comment -

        > The only time it would seem to matter is when the user wants to specify a different data store (from the default) for the credentials database.

        that is the use case and that seemed reasonable to me for having different data stores. i'm not sure i'd want to mix the auth database with my application data - or i'd at least want the option to not do so. note that this will still be possible even without the credentialsDbLocation, but we just won't have that extra setting for folks to worry about.

        Show
        spmallette stephen mallette added a comment - > The only time it would seem to matter is when the user wants to specify a different data store (from the default) for the credentials database. that is the use case and that seemed reasonable to me for having different data stores. i'm not sure i'd want to mix the auth database with my application data - or i'd at least want the option to not do so. note that this will still be possible even without the credentialsDbLocation , but we just won't have that extra setting for folks to worry about.
        Hide
        jeromatron Jeremy Hanna added a comment -

        I would say +1 to this. It's confusing to have to specify the properties file to the data store again in the authentication configuration. The only time it would seem to matter is when the user wants to specify a different data store (from the default) for the credentials database. I'm not sure when that would be a reasonable use case. Perhaps if they are storing several graphs in different data stores using different configs and they want a shared central credentials db? You would know better about those use cases though stephen mallette.

        Show
        jeromatron Jeremy Hanna added a comment - I would say +1 to this. It's confusing to have to specify the properties file to the data store again in the authentication configuration. The only time it would seem to matter is when the user wants to specify a different data store (from the default) for the credentials database. I'm not sure when that would be a reasonable use case. Perhaps if they are storing several graphs in different data stores using different configs and they want a shared central credentials db? You would know better about those use cases though stephen mallette .

          People

          • Assignee:
            spmallette stephen mallette
            Reporter:
            spmallette stephen mallette
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development