Uploaded image for project: 'Avro'
  1. Avro
  2. AVRO-1213

Revisit dependencies on Jetty, servlet-api, and Netty

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Blocker
    • Resolution: Unresolved
    • Affects Version/s: 1.7.2
    • Fix Version/s: 1.9.0
    • Component/s: java
    • Labels:
      None

      Description

      The compile scoped dependency on jetty servlet-api in the IPC pom file can be problematic if using Avro in a webapp environment. Would it be possible to make this dependency either optional or provided? Or maybe Avro modularize into sub-modules in such a way that desired features can be assembled piecemeal?

        Issue Links

          Activity

          Hide
          cutting Doug Cutting added a comment -

          Optional sounds reasonable to me. It's hard to use a servlet without having that provided and we shouldn't mandate a particular source for the servlet api.

          And shouldn't the other Jetty dependencies be for tests only?

          Show
          cutting Doug Cutting added a comment - Optional sounds reasonable to me. It's hard to use a servlet without having that provided and we shouldn't mandate a particular source for the servlet api. And shouldn't the other Jetty dependencies be for tests only?
          Hide
          scott_carey Scott Carey added a comment -

          Optional is OK for now for Jetty, and Netty. The use can choose which they want. Optional may require changing avro-tools' pom to declare those, and we'll need to document the change. I'd say it would need to wait until 1.8.0 because upgrading would break current projects, and there is a work-around for users now to exclude jetty and/or the servlet downstream dependencies in their pom.

          Longer term, we could break up IPC into more modules, one for HTTP RPC and another for socket, since the two have very different downstream dependencies.

          Show
          scott_carey Scott Carey added a comment - Optional is OK for now for Jetty, and Netty. The use can choose which they want. Optional may require changing avro-tools' pom to declare those, and we'll need to document the change. I'd say it would need to wait until 1.8.0 because upgrading would break current projects, and there is a work-around for users now to exclude jetty and/or the servlet downstream dependencies in their pom. Longer term, we could break up IPC into more modules, one for HTTP RPC and another for socket, since the two have very different downstream dependencies.
          Hide
          saden1 Sharmarke Aden added a comment -

          Doug,

          I wasn't really sure in what capacity Jetty is used but looking at IPC code it looks like those jetty dependencies are needed down stream by those wishing to use IPC.

          Scott,

          I think you are right on them money. Ultimately what you want to do is break the IPC into Jetty and Netty sub-modules so they can become optional and allow of future growth with respect to adding new Server implementations (i.e. tomcat, vert.x, NodeJS, etc).

          Show
          saden1 Sharmarke Aden added a comment - Doug, I wasn't really sure in what capacity Jetty is used but looking at IPC code it looks like those jetty dependencies are needed down stream by those wishing to use IPC. Scott, I think you are right on them money. Ultimately what you want to do is break the IPC into Jetty and Netty sub-modules so they can become optional and allow of future growth with respect to adding new Server implementations (i.e. tomcat, vert.x, NodeJS, etc).
          Hide
          scott_carey Scott Carey added a comment -

          Netty also now has HTTP support, so we may be able to consolidate significantly and use it for both.

          Show
          scott_carey Scott Carey added a comment - Netty also now has HTTP support, so we may be able to consolidate significantly and use it for both.
          Hide
          chengas123 Ben McCann added a comment -

          The version of Jetty used is ancient (8 years old!!) This dependency in Avro IPC is causing me major problems because it's pulling in such an old version of the servlet API. We should really upgrade to a newer version like org.eclipse.jetty:jetty-server:9.3.8.v20160314. Or just drop the jetty server and use the netty server since there's no real need for two of them and it just adds overhead to maintain two implementations.

          Show
          chengas123 Ben McCann added a comment - The version of Jetty used is ancient (8 years old!!) This dependency in Avro IPC is causing me major problems because it's pulling in such an old version of the servlet API. We should really upgrade to a newer version like org.eclipse.jetty:jetty-server:9.3.8.v20160314. Or just drop the jetty server and use the netty server since there's no real need for two of them and it just adds overhead to maintain two implementations.
          Hide
          rdblue Ryan Blue added a comment -

          Ben McCann, that sounds fine to me. What is the benefit of having both? Should we update the Netty version?

          Show
          rdblue Ryan Blue added a comment - Ben McCann , that sounds fine to me. What is the benefit of having both? Should we update the Netty version?
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user dkulp opened a pull request:

          https://github.com/apache/avro/pull/244

          AVRO-1213 Update to latest release of Jetty

          This updates the Jetty dependency to the latest Jetty release.

          Note: this does not address the Jetty vs. Netty thing or updates to the latest Netty. I hope to tackle that soon, but the netty update is huge/hard with a much larger impact.

          The API signatures do change slightly, but that is obviously required due to the org.mortbay -> org.eclipse change. However, getting onto the supported version of Jetty (with the latest security updates and fixes) is important.

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

          $ git pull https://github.com/dkulp/avro JETTY

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

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


          commit b9ea20ba5700a7df60411a7bd62406f5c4beab42
          Author: Daniel Kulp <dkulp@apache.org>
          Date: 2017-09-08T17:18:33Z

          AVRO-1213 Update to latest release of Jetty


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user dkulp opened a pull request: https://github.com/apache/avro/pull/244 AVRO-1213 Update to latest release of Jetty This updates the Jetty dependency to the latest Jetty release. Note: this does not address the Jetty vs. Netty thing or updates to the latest Netty. I hope to tackle that soon, but the netty update is huge/hard with a much larger impact. The API signatures do change slightly, but that is obviously required due to the org.mortbay -> org.eclipse change. However, getting onto the supported version of Jetty (with the latest security updates and fixes) is important. You can merge this pull request into a Git repository by running: $ git pull https://github.com/dkulp/avro JETTY Alternatively you can review and apply these changes as the patch at: https://github.com/apache/avro/pull/244.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 #244 commit b9ea20ba5700a7df60411a7bd62406f5c4beab42 Author: Daniel Kulp <dkulp@apache.org> Date: 2017-09-08T17:18:33Z AVRO-1213 Update to latest release of Jetty

            People

            • Assignee:
              Unassigned
              Reporter:
              saden1 Sharmarke Aden
            • Votes:
              3 Vote for this issue
              Watchers:
              11 Start watching this issue

              Dates

              • Created:
                Updated:

                Development