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

Make Graphite and Ganglia optional dependencies

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Done
    • Affects Version/s: 3.2.3
    • Fix Version/s: 3.3.0
    • Component/s: server
    • Labels:
      None

      Description

      This ticket started out as "Provide a way to not specifically depend on and package the various reporters tied to the Gremlin Server `MetricsManager`", but while that is a nice idea, it ends up being more complicated than what is required and at this point I haven't really heard of any users asking for the ability to drop in other reporters.

      Anyway, Ganglia/Graphite should be made optional dependencies as they aren't really required for operation of Gremlin Server and it should be up to the user to decide which they want to use and include. Plus, this gets rid of the ugly problem of an LGPL dependency which we used to just exclude and thus force the user to copy in manually. As a result, we kinda added stuff to the distribution which didn't work out of the box anyway. Better to just avoid the whole issue and not include them.

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

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

          Github user okram commented on the issue:

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

          VOTE +1

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

          Github user dkuppitz commented on the issue:

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

          VOTE: +1

          Show
          githubbot ASF GitHub Bot added a comment - Github user dkuppitz commented on the issue: https://github.com/apache/tinkerpop/pull/628 VOTE: +1
          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/628#discussion_r123490358

          — Diff: pom.xml —
          @@ -597,6 +597,7 @@ limitations under the License.
          <groupId>com.codahale.metrics</groupId>
          <artifactId>metrics-graphite</artifactId>
          <version>$

          {metrics.version}

          </version>
          + <optional>true</optional>
          — End diff –

          Doh, I probably only focused on the addition of the optional tag and assumed it's the same dependency block as above. All good here.

          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/628#discussion_r123490358 — Diff: pom.xml — @@ -597,6 +597,7 @@ limitations under the License. <groupId>com.codahale.metrics</groupId> <artifactId>metrics-graphite</artifactId> <version>$ {metrics.version} </version> + <optional>true</optional> — End diff – Doh, I probably only focused on the addition of the optional tag and assumed it's the same dependency block as above. All good here.
          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/628#discussion_r123489325

          — Diff: pom.xml —
          @@ -597,6 +597,7 @@ limitations under the License.
          <groupId>com.codahale.metrics</groupId>
          <artifactId>metrics-graphite</artifactId>
          <version>$

          {metrics.version}

          </version>
          + <optional>true</optional>
          — End diff –

          I don't follow - the `<version>` is there. It's presented as a pom variable, but it's defined.

          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/628#discussion_r123489325 — Diff: pom.xml — @@ -597,6 +597,7 @@ limitations under the License. <groupId>com.codahale.metrics</groupId> <artifactId>metrics-graphite</artifactId> <version>$ {metrics.version} </version> + <optional>true</optional> — End diff – I don't follow - the `<version>` is there. It's presented as a pom variable, but it's defined.
          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/628#discussion_r123489196

          — Diff: gremlin-server/pom.xml —
          @@ -63,10 +63,12 @@ limitations under the License.
          <dependency>
          <groupId>com.codahale.metrics</groupId>
          <artifactId>metrics-graphite</artifactId>
          + <optional>true</optional>
          — End diff –

          All of metrics is under `<dependencyManagement>` in the root pom.xml so the version is defined there:

          https://github.com/apache/tinkerpop/blob/268333b4778c87ce6c2429eda97e3e1adc7757e3/pom.xml#L602-L642

          I don't think it needs to be specified again.

          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/628#discussion_r123489196 — Diff: gremlin-server/pom.xml — @@ -63,10 +63,12 @@ limitations under the License. <dependency> <groupId>com.codahale.metrics</groupId> <artifactId>metrics-graphite</artifactId> + <optional>true</optional> — End diff – All of metrics is under `<dependencyManagement>` in the root pom.xml so the version is defined there: https://github.com/apache/tinkerpop/blob/268333b4778c87ce6c2429eda97e3e1adc7757e3/pom.xml#L602-L642 I don't think it needs to be specified again.
          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/628#discussion_r123485417

          — Diff: pom.xml —
          @@ -597,6 +597,7 @@ limitations under the License.
          <groupId>com.codahale.metrics</groupId>
          <artifactId>metrics-graphite</artifactId>
          <version>$

          {metrics.version}

          </version>
          + <optional>true</optional>
          — End diff –

          Same here. It's probably better to have the version tag included.

          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/628#discussion_r123485417 — Diff: pom.xml — @@ -597,6 +597,7 @@ limitations under the License. <groupId>com.codahale.metrics</groupId> <artifactId>metrics-graphite</artifactId> <version>$ {metrics.version} </version> + <optional>true</optional> — End diff – Same here. It's probably better to have the version tag included.
          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/628#discussion_r123484932

          — Diff: gremlin-server/pom.xml —
          @@ -63,10 +63,12 @@ limitations under the License.
          <dependency>
          <groupId>com.codahale.metrics</groupId>
          <artifactId>metrics-graphite</artifactId>
          + <optional>true</optional>
          — End diff –

          Shouldn't this include the version? Asking because you've explicitly mentioned 3.0.2 before.

          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/628#discussion_r123484932 — Diff: gremlin-server/pom.xml — @@ -63,10 +63,12 @@ limitations under the License. <dependency> <groupId>com.codahale.metrics</groupId> <artifactId>metrics-graphite</artifactId> + <optional>true</optional> — End diff – Shouldn't this include the version? Asking because you've explicitly mentioned 3.0.2 before.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user spmallette opened a pull request:

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

          TINKERPOP-1550 Made ganglia/graphite optional dependencies

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

          These dependencies must now be installed manually by users. Updated docs and the LICENSE for Gremlin Server. Gets rid of the weird one-off exclusion of an LGPL license (couldn't support Ganglia out of the box anyway) and lessens transitive dependency burdens.

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

          VOTE +1

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

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

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

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


          commit 945c23571fcccafb95f7c129d368955cd068c430
          Author: Stephen Mallette <spmva@genoprime.com>
          Date: 2017-06-15T16:11:51Z

          TINKERPOP-1550 Made ganglia/graphite optional dependencies

          These dependencies must now be installed manually by users. Updated docs and the LICENSE for Gremlin Server.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user spmallette opened a pull request: https://github.com/apache/tinkerpop/pull/628 TINKERPOP-1550 Made ganglia/graphite optional dependencies https://issues.apache.org/jira/browse/TINKERPOP-1550 These dependencies must now be installed manually by users. Updated docs and the LICENSE for Gremlin Server. Gets rid of the weird one-off exclusion of an LGPL license (couldn't support Ganglia out of the box anyway) and lessens transitive dependency burdens. Builds with `mvn clean install && mvn verify -pl gremlin-server -DskipIntegrationTests=false && mvn verify -pl gremlin-console -DskipIntegrationTests=false` VOTE +1 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1550 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/628.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 #628 commit 945c23571fcccafb95f7c129d368955cd068c430 Author: Stephen Mallette <spmva@genoprime.com> Date: 2017-06-15T16:11:51Z TINKERPOP-1550 Made ganglia/graphite optional dependencies These dependencies must now be installed manually by users. Updated docs and the LICENSE for Gremlin Server.

            People

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

              Dates

              • Created:
                Updated:
                Resolved:

                Development