Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.2
    • Fix Version/s: 0.3
    • Component/s: None
    • Labels:
      None

      Description

      Further dependency cleanup is required, mainly to set the right hadoop dependency for mahout-math and fix exclusions for the hadoop dependency in the parent pom. Other minor cleanups too.

      The patch includes the following changes:

      maven (parent pom)

      • added inceptionYear (2008)
      • removed some exclusions for hadoop dependency: avro, commons-codec, commons-httpclient in the dependendy management section.
      • removed javax.mail dependency

      mahout-math

      • switched from o.a.m.hadoop:hadoop-core dependency to new o.a.hadoop:hadoop-core dependency used in core, version specified in dependencyManagement section of parent pom.
      • removed unnecessary compile scope from gson dependency

      mahout-core

      • removed: kfs, jets3t, xmlenc, unused, originally added to support old o.a.mahout.hadoop:hadoop-core:0.20.1 dependency
      • removed: commons-httpclient, now added transitively from new o.a.hadoop:hadoop-core:0.20.2-SNAPSHOT dependency
      • set slf4j-jcl to test scope.
      • removed: watchmaker-swing, added later in mahout-examples where it is actually used.
      • fixed uncommons-maths groupId
      • removed unused lucene-analyzers dependency.
      • added easymock dependencies explicitly

      mahout-utils

      • removed unused easymock dependencies

      mahout-examples

      • added watchmaker-framework and watchmaker-swing
      1. MAHOUT-238.patch
        8 kB
        Drew Farris
      2. MAHOUT-238.patch
        8 kB
        Drew Farris

        Activity

        Hide
        Drew Farris added a comment -

        patch added

        Show
        Drew Farris added a comment - patch added
        Hide
        Sean Owen added a comment -

        For some reason I can't apply the patch but again suspect it's a local problem. I'm about to just blow this all away and start over.

        My only question from visually inspecting the patch is: we shouldn't directly depend on commons-logging right? we log via SLF4J only.

        Show
        Sean Owen added a comment - For some reason I can't apply the patch but again suspect it's a local problem. I'm about to just blow this all away and start over. My only question from visually inspecting the patch is: we shouldn't directly depend on commons-logging right? we log via SLF4J only.
        Hide
        Drew Farris added a comment -

        For some reason I can't apply the patch but again suspect it's a local problem. I'm about to just blow this all away and start over.

        Does is apply cleanly to a fresh checkout when patching from the command-line? If you give it another try and it still fails, I'll look into it further.

        we shouldn't directly depend on commons-logging right? we log via SLF4J only.

        Right, we log via the slf4j api, but at runtime, the slf4j uses commons-logging to do its work. slf4j-api -> slf4j-jcl -> commons-logging.

        I'm not exactly sure why that particular mechanism was chosen for mahout, but the most common case for that route is when one of our dependencies depend on commons-logging and it needs to be in the classpath anyway.

        That aside, it would probably make sense to change the scope of commons-logging to runtime. It is certainly not needed for compilation. After thinking about this a bit, I suspect I probably need to change slf4j-jcl to a runtime dependency as well (as opposed to a test dependency). This way, they are there as dependencies if they are needed, but if someone using mahout wants to use a different framework they can be excluded.

        Show
        Drew Farris added a comment - For some reason I can't apply the patch but again suspect it's a local problem. I'm about to just blow this all away and start over. Does is apply cleanly to a fresh checkout when patching from the command-line? If you give it another try and it still fails, I'll look into it further. we shouldn't directly depend on commons-logging right? we log via SLF4J only. Right, we log via the slf4j api, but at runtime, the slf4j uses commons-logging to do its work. slf4j-api -> slf4j-jcl -> commons-logging. I'm not exactly sure why that particular mechanism was chosen for mahout, but the most common case for that route is when one of our dependencies depend on commons-logging and it needs to be in the classpath anyway. That aside, it would probably make sense to change the scope of commons-logging to runtime. It is certainly not needed for compilation. After thinking about this a bit, I suspect I probably need to change slf4j-jcl to a runtime dependency as well (as opposed to a test dependency). This way, they are there as dependencies if they are needed, but if someone using mahout wants to use a different framework they can be excluded.
        Hide
        Sean Owen added a comment -

        A complete delete and reinstall fixed everything.
        Yes I agree with your assessment of how the dependencies should be arranged.
        Let me know when I can help commit.

        Show
        Sean Owen added a comment - A complete delete and reinstall fixed everything. Yes I agree with your assessment of how the dependencies should be arranged. Let me know when I can help commit.
        Hide
        Drew Farris added a comment -

        revised patch – slf4j-jcl and commons-logging set to runtime dependencies.

        Ready for commit.

        Show
        Drew Farris added a comment - revised patch – slf4j-jcl and commons-logging set to runtime dependencies. Ready for commit.
        Hide
        Sean Owen added a comment -

        Looks good and works for me. Resolving after committing.

        Show
        Sean Owen added a comment - Looks good and works for me. Resolving after committing.
        Hide
        Drew Farris added a comment -

        Thanks Sean.

        Show
        Drew Farris added a comment - Thanks Sean.

          People

          • Assignee:
            Drew Farris
            Reporter:
            Drew Farris
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development