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

ServerGremlinExecutor construction need not use generics for ExecutorService

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Done
    • Affects Version/s: 3.1.0-incubating
    • Fix Version/s: 3.3.0
    • Component/s: server
    • Labels:

      Description

      See TINKERPOP3-912 for more information but the ServerGremlinExecutor will always need be bound to Netty so there's not much need for use of generics in the class definition. Remove unecessary constructors and perhaps scope them internal as this class will not be publicly constructed. This will be a breaking change.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user spmallette opened a pull request:

          https://github.com/apache/tinkerpop/pull/638

          TINKERPOP-999 Removed generics from ServerGremlinExecutor

          https://issues.apache.org/jira/browse/TINKERPOP-999

          Pretty simple - I probably could have CTR'd this but since it was a breaking change on 3.3.0 I wanted folks to be more aware of it in case someone was using this class in some way I'm not imagining.

          Builds with `mvn clean install -DskipTests && mvn verify -pl gremlin-server -DskipIntegrationTests=false`

          VOTE +1

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

          $ git pull https://github.com/apache/tinkerpop TINKERPOP-999

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

          https://github.com/apache/tinkerpop/pull/638.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 #638


          commit 4c89e1594537f02c4bb3f0efa60f3e5042621442
          Author: Stephen Mallette <spmva@genoprime.com>
          Date: 2017-06-20T19:14:44Z

          TINKERPOP-999 Removed generics from ServerGremlinExecutor


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/tinkerpop/pull/638 TINKERPOP-999 Removed generics from ServerGremlinExecutor https://issues.apache.org/jira/browse/TINKERPOP-999 Pretty simple - I probably could have CTR'd this but since it was a breaking change on 3.3.0 I wanted folks to be more aware of it in case someone was using this class in some way I'm not imagining. Builds with `mvn clean install -DskipTests && mvn verify -pl gremlin-server -DskipIntegrationTests=false` VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-999 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/638.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 #638 commit 4c89e1594537f02c4bb3f0efa60f3e5042621442 Author: Stephen Mallette <spmva@genoprime.com> Date: 2017-06-20T19:14:44Z TINKERPOP-999 Removed generics from ServerGremlinExecutor
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tinkerpop/pull/638#discussion_r123119092

          — Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java —
          @@ -108,54 +108,24 @@ public GremlinServer(final Settings settings)

          { workerGroup = new NioEventLoopGroup(settings.threadPoolWorker, threadFactoryWorker); }
          • serverGremlinExecutor = new ServerGremlinExecutor<>(settings, null, workerGroup, EventLoopGroup.class);
            + serverGremlinExecutor = new ServerGremlinExecutor(settings, null, workerGroup, null);
              • End diff –

          Looks like the 4th parameter is no longer needed. Since the PR targets `master/`, it would be fine to remove it altogether, no?

          Show
          githubbot ASF GitHub Bot added a comment - Github user dkuppitz commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/638#discussion_r123119092 — Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java — @@ -108,54 +108,24 @@ public GremlinServer(final Settings settings) { workerGroup = new NioEventLoopGroup(settings.threadPoolWorker, threadFactoryWorker); } serverGremlinExecutor = new ServerGremlinExecutor<>(settings, null, workerGroup, EventLoopGroup.class); + serverGremlinExecutor = new ServerGremlinExecutor(settings, null, workerGroup, null); End diff – Looks like the 4th parameter is no longer needed. Since the PR targets `master/`, it would be fine to remove it altogether, no?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tinkerpop/pull/638#discussion_r123125481

          — Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java —
          @@ -108,54 +108,24 @@ public GremlinServer(final Settings settings)

          { workerGroup = new NioEventLoopGroup(settings.threadPoolWorker, threadFactoryWorker); }
          • serverGremlinExecutor = new ServerGremlinExecutor<>(settings, null, workerGroup, EventLoopGroup.class);
            + serverGremlinExecutor = new ServerGremlinExecutor(settings, null, workerGroup, null);
              • End diff –

          i think you're right. I sorta wonder if the second parameter is of value. Gremlin Server is the only thing that really creates that ExecutorService.

          Show
          githubbot ASF GitHub Bot added a comment - Github user spmallette commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/638#discussion_r123125481 — Diff: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java — @@ -108,54 +108,24 @@ public GremlinServer(final Settings settings) { workerGroup = new NioEventLoopGroup(settings.threadPoolWorker, threadFactoryWorker); } serverGremlinExecutor = new ServerGremlinExecutor<>(settings, null, workerGroup, EventLoopGroup.class); + serverGremlinExecutor = new ServerGremlinExecutor(settings, null, workerGroup, null); End diff – i think you're right. I sorta wonder if the second parameter is of value. Gremlin Server is the only thing that really creates that ExecutorService.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user okram commented on the issue:

          https://github.com/apache/tinkerpop/pull/638

          VOTE +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user okram commented on the issue: https://github.com/apache/tinkerpop/pull/638 VOTE +1
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tinkerpop/pull/638

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

            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