Uploaded image for project: 'Pig'
  1. Pig
  2. PIG-5298

Verify if org.mortbay.jetty is removable

    Details

    • Type: Improvement
    • Status: Patch Available
    • Priority: Major
    • Resolution: Unresolved
    • Affects Version/s: 0.17.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      Although we pull in jetty libraries in ivy Pig does not depend on org.mortbay.jetty explicitly. The only exception I see is in Piggybank where I think this can be swapped by javax.el-api and log4j.

      We should investigate (check build, run unit tests across all exec modes) and remove if it turns out to be unnecessary.

      1. PIG-5298_1.patch
        7 kB
        Nandor Kollar
      2. PIG-5298_2.patch
        6 kB
        Nandor Kollar
      3. PIG-5298_3.patch
        6 kB
        Nandor Kollar

        Activity

        Hide
        szita Adam Szita added a comment -

        Thanks for looking into this Nandor Kollar.

        Few comments:

        • please list an entry for the tomcat version (9.0.0.M26) in libraries.properties instead of ivy.xml
        • list the tomcat dependencies in the pom templates for pig and piggybank
        Show
        szita Adam Szita added a comment - Thanks for looking into this Nandor Kollar . Few comments: please list an entry for the tomcat version (9.0.0.M26) in libraries.properties instead of ivy.xml list the tomcat dependencies in the pom templates for pig and piggybank
        Hide
        rohini Rohini Palaniswamy added a comment -

        Why do we need a dependency on tomcat? Is it for the EL implementation? Can't we just use jetty but avoid copying it to lib (like maven scope provided) and instead use the jetty that comes with hadoop or spark?

        Show
        rohini Rohini Palaniswamy added a comment - Why do we need a dependency on tomcat? Is it for the EL implementation? Can't we just use jetty but avoid copying it to lib (like maven scope provided) and instead use the jetty that comes with hadoop or spark?
        Hide
        nkollar Nandor Kollar added a comment - - edited

        Rohini Palaniswamy it seems that Jetty is used only in Piggybank, and like you said, it is used just for its EL implementation. I'm not familiar with the history of Jetty, but it seems that they had their own EL implementation (or at least they had an artifact which included an EL implementation), and at one point they decided to use Glassfish's EL implementation. Depending on such an old Jetty version is not the best I think, it is already deprecated. Since it seems Pig only needs an EL implementation, I thought we can use Tomcat's EL instead of pulling the entire Jetty server. What do you think? Should we still pull Jetty (or maybe just Jetty JSP module would be enough), use Tomcat's EL, or use Glassfish's EL, like Jetty does?

        Show
        nkollar Nandor Kollar added a comment - - edited Rohini Palaniswamy it seems that Jetty is used only in Piggybank, and like you said, it is used just for its EL implementation. I'm not familiar with the history of Jetty, but it seems that they had their own EL implementation (or at least they had an artifact which included an EL implementation), and at one point they decided to use Glassfish's EL implementation. Depending on such an old Jetty version is not the best I think, it is already deprecated . Since it seems Pig only needs an EL implementation, I thought we can use Tomcat's EL instead of pulling the entire Jetty server. What do you think? Should we still pull Jetty (or maybe just Jetty JSP module would be enough), use Tomcat's EL, or use Glassfish's EL, like Jetty does?
        Hide
        szita Adam Szita added a comment -

        Nandor Kollar, if currently the only reason the pull in jetty is to make use of its EL implementation which as you say in the newer versions is basically borrowed from that of Glassfish the logical thing to do IMHO would be to use just glassfish EL, no jetty and no tomcat.

        I believe we should always try to reduce the number and size of dependent libraries to only those that we actually make use of.

        Show
        szita Adam Szita added a comment - Nandor Kollar , if currently the only reason the pull in jetty is to make use of its EL implementation which as you say in the newer versions is basically borrowed from that of Glassfish the logical thing to do IMHO would be to use just glassfish EL, no jetty and no tomcat. I believe we should always try to reduce the number and size of dependent libraries to only those that we actually make use of.
        Hide
        rohini Rohini Palaniswamy added a comment -

        the logical thing to do IMHO would be to use just glassfish EL

        Sounds good. We have been moving away from tomcat in most hadoop projects (For eg: Oozie). Jetty has kind of become the standard in all hadoop projects. So better to stick to it or what it uses as we would get to know quickly if there are any security issues.

        Show
        rohini Rohini Palaniswamy added a comment - the logical thing to do IMHO would be to use just glassfish EL Sounds good. We have been moving away from tomcat in most hadoop projects (For eg: Oozie). Jetty has kind of become the standard in all hadoop projects. So better to stick to it or what it uses as we would get to know quickly if there are any security issues.
        Hide
        nkollar Nandor Kollar added a comment -

        Attached PIG-5298_2.patch, with Glassfish's EL implementation.

        Show
        nkollar Nandor Kollar added a comment - Attached PIG-5298 _2.patch, with Glassfish's EL implementation.

          People

          • Assignee:
            nkollar Nandor Kollar
            Reporter:
            szita Adam Szita
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:

              Development