You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@accumulo.apache.org by GitBox <gi...@apache.org> on 2020/04/07 16:24:45 UTC

[GitHub] [accumulo-docker] karthick-rn opened a new pull request #12: Update Accumulo version in Dockerfile & README.md

karthick-rn opened a new pull request #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12
 
 
   In this PR, I have updated the Accumulo version along with Hadoop & Zookeeper to match with the latest releases. With these changes, I'm able to successfully build the image. 
   Also, updated the README.md. Once you're happy with these changes, may be I can push the image to `apache/accumulo` at DockerHub. Thanks
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611516281
 
 
   Looks like the order of the classpath is important here. When adding `ENV CLASSPATH /opt/zookeeper/lib/*` to Dockerfile, Accumulo positions the `ENV` directive first in the classpath like below. 
   <pre>
   <b>/opt/zookeeper/lib/*</b>:/opt/accumulo/conf:/opt/accumulo/lib/*:/opt/hadoop/etc/hadoop:/opt/zookeeper/*:/opt/hadoop/share/hadoop/client/* </pre>
   This has resulted in the below exception when trying to run queries on ashell.
   `Exception in thread "shell" java.lang.NoSuchMethodError: org.apache.commons.cli.Options.hasShortOption(Ljava/lang/String;)Z` 
   
   To maintain the order of the classpath, we tried several options & this one with `sed` works and doesn't require the post-script. I have updated the PR with the new changes, please review and let me know your thoughts/suggestions. Thanks
   
   
   
    
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-610572008
 
 
   I am getting a NoClassDefFoundError related to zookeeper when trying to run the command.  
   ```
   java.util.ServiceConfigurationError: org.apache.accumulo.start.spi.KeywordExecutable: Provider org.apache.accumulo.server.init.Initialize could not be instantiated
   	at java.util.ServiceLoader.fail(ServiceLoader.java:232)
   	at java.util.ServiceLoader.access$100(ServiceLoader.java:185)
   	at java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:384)
   	at java.util.ServiceLoader$LazyIterator.next(ServiceLoader.java:404)
   	at java.util.ServiceLoader$1.next(ServiceLoader.java:480)
   	at org.apache.accumulo.start.Main.checkDuplicates(Main.java:243)
   	at org.apache.accumulo.start.Main.getExecutables(Main.java:234)
   	at org.apache.accumulo.start.Main.main(Main.java:88)
   Caused by: java.lang.NoClassDefFoundError: org/apache/zookeeper/KeeperException
   	at java.lang.Class.getDeclaredConstructors0(Native Method)
   	at java.lang.Class.privateGetDeclaredConstructors(Class.java:2671)
   	at java.lang.Class.getConstructor0(Class.java:3075)
   	at java.lang.Class.newInstance(Class.java:412)
   	at java.util.ServiceLoader$LazyIterator.nextService(ServiceLoader.java:380)
   	... 5 more
   Caused by: java.lang.ClassNotFoundException: org.apache.zookeeper.KeeperException
   	at java.net.URLClassLoader.findClass(URLClassLoader.java:382)
   	at java.lang.ClassLoader.loadClass(ClassLoader.java:419)
   	at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:352)
   	at java.lang.ClassLoader.loadClass(ClassLoader.java:352)
   	... 10 more
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611677865
 
 
   > We should vote on a release for this, and then publish it to Dockerhub as described in the README (I think INFRA can help with that). If anyone is interested in pursuing this, please follow up on the dev mailing list.
   
   @ctubbsii I'm interested, will follow-up on the dev mailing list. Thanks

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611158745
 
 
   > @karthick-rn I wonder if adding `ENV CLASSPATH /opt/zookeeper/lib/*` towards the end of the dockerfile instead of adding the accumulo-env.sh file would work. This should be picked up by Accumulo env, the only drawback is that we dont get control over the order of where `/opt/zookeeper/lib/*` is in the classpath.
   
   Thanks for the suggestion @keith-turner. I'm trying this now, will update soon

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-610625411
 
 
   @milleruntime Regarding the exception, can you add the `/lib` directory as shown below to the classpath & try running the same command. 
   <pre>
   CLASSPATH="${CLASSPATH}:${lib}/*:${HADOOP_CONF_DIR}:${ZOOKEEPER_HOME}/*:${HADOOP_HOME}/share/hadoop/client/*<b>:${ZOOKEEPER_HOME}/lib/*</b>"
   </pre>

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611079542
 
 
   > OK I forgot the fixes for the newer version of ZK haven't been released yet. I think the dockerfile should work out of the box with the last release of Accumulo, version 2.0.0. So the ZK version should be 3.4.14 or if it works with 3.5.7 then that is fine.
   
   It would have the same problems with 3.5.7. So, it should probably stay 3.4.14, or have some post-script to modify the class path in the Accumulo env script.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611182019
 
 
   > My only concern with this change is that we are creating more work for when we release 2.1. But if I understand the updates correctly, that won't be a problem since we would have to update the path to the new ZK binaries anyway.
   
   Yeah, this might actually be saving us work (because of the tarball renames). If @keith-turner 's suggestion to use the `ENV` directive works, then the only change we'd need to do for 2.1 is to drop that superfluous `ENV` line.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] keith-turner commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
keith-turner commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611144837
 
 
   @karthick-rn I wonder if adding `ENV CLASSPATH /opt/zookeeper/lib/*` towards the end of the dockerfile instead of adding the accumulo-env.sh file would work.  This should be picked up by Accumulo env, the only drawback is that we dont get control over the order of where `/opt/zookeeper/lib/*` is in the classpath.   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611173695
 
 
   > > @milleruntime I'm not following your use of "out-of-the-box" here.
   > 
   > A developer should be able to check out accumulo-docker, follow the README and build and run a 2.0.0 container. If they require additional steps like setting the classpath to work then we shouldn't bake the version in the dockerfile. The versions in the dockerfile should work without extra steps.
   
   @milleruntime Oh, okay. So, I think you mean `accumulo-docker`, "out-of-the-box", should work. I previously thought you meant something like `accumulo-docker` should use "out-of-the-box" `accumulo`. If you mean the former, then I agree. But, all of the proposed changes in this PR satisfy that, so I don't see the problem.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611112401
 
 
   @karthick-rn Since this change is really to make docker work for version 2.1.0, if you want to keep this PR open for once we release 2.1.0 that is fine.  We can make a separate change for now to simply update the version from alpha to 2.0.0. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611163540
 
 
   > @milleruntime I'm not following your use of "out-of-the-box" here.
   
   A developer should be able to check out accumulo-docker, follow the README and build and run a 2.0.0 container.  If they require additional steps like setting the classpath to work then we shouldn't bake the version in the dockerfile.  The versions in the dockerfile should work without extra steps.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii edited a comment on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii edited a comment on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611177039
 
 
   Since @milleruntime mentioned the README, it's worth noting that this change would make some versions of ZooKeeper not work at all (3.4 and earlier; mainly because ZooKeeper renamed their tarballs). However, without this change, the newer versions won't work. I'm inclined to prioritize the newer versions.
   
   In either case, the examples in the README should avoid using versions that won't work, and should mention that some combinations of ZooKeeper and Hadoop versions may not work.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611177039
 
 
   Since @milleruntime mentioned the README, it's worth noting that this change would make some versions of ZooKeeper not work at all (3.4 and earlier). However, without this change, the newer versions won't work. I'm inclined to prioritize the newer versions.
   
   In either case, the examples in the README should avoid using versions that won't work, and should mention that some combinations of ZooKeeper and Hadoop versions may not work.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-610497372
 
 
   @karthick-rn thanks for the updates!  I want to give the updates a try on my local dev environment but I recently changed from fedora to ubuntu so I need to get docker setup first.  

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611135081
 
 
   
   
   
   > @karthick-rn Since this change is really to make docker work for version 2.1.0, if you want to keep this PR open for once we release 2.1.0 that is fine. We can make a separate change for now to simply update the version from alpha to 2.0.0.
   
   @milleruntime Sorry if I wasn't clear. This change is to make docker to work with Accumulo 2.0 & not 2.1. The reason for having the `accumulo-env.sh` as a post-script is to address the classpath issue in Accumulo 2.0 to work with ZK versions 3.5.x & above.  This classpath issue has been fixed in Accumulo 2.1 & we no longer need the post-script when we are running docker on Accumulo 2.1.  Let me know if you are seeing any issues in Dockerfile running Accumulo 2.0? Also, if you still think you might need ZK 3.4.14 I'm happy to make the changes. Thanks

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611178810
 
 
   My only concern with this change is that we are creating more work for when we release 2.1.  But if I understand the updates correctly, that won't be a problem since we would have to update the path to the new ZK binaries anyway.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on a change in pull request #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on a change in pull request #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#discussion_r405112879
 
 

 ##########
 File path: README.md
 ##########
 @@ -18,9 +16,9 @@ While it is easier to pull from DockerHub, the image will default to the softwar
 
 | Software    | Version       |
 |-------------|---------------|
-| [Accumulo]  | 2.0.0-alpha-1 |
-| [Hadoop]    | 3.1.1         |
-| [Zookeeper] | 3.4.13        |
+| [Accumulo]  | 2.0.0         |
+| [Hadoop]    | 3.2.1         |
+| [Zookeeper] | 3.6.0         |
 
 Review comment:
   I have addressed this in the new commit. Thanks 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii commented on a change in pull request #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on a change in pull request #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#discussion_r404966531
 
 

 ##########
 File path: README.md
 ##########
 @@ -18,9 +16,9 @@ While it is easier to pull from DockerHub, the image will default to the softwar
 
 | Software    | Version       |
 |-------------|---------------|
-| [Accumulo]  | 2.0.0-alpha-1 |
-| [Hadoop]    | 3.1.1         |
-| [Zookeeper] | 3.4.13        |
+| [Accumulo]  | 2.0.0         |
+| [Hadoop]    | 3.2.1         |
+| [Zookeeper] | 3.6.0         |
 
 Review comment:
   The ZooKeeper trademark has an upper-case `Z` and `K`. We make that mistake in a lot of places, but I try to call it out whenever I see it in a new code change.
   
   ```suggestion
   | [ZooKeeper] | 3.6.0         |
   ```
   
   The corresponding markdown link label would also need to change.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611091320
 
 
   > It would have the same problems with 3.5.7. So, it should probably stay 3.4.14, or have some post-script to modify the class path in the Accumulo env script.
   
   I discussed this with @milleruntime on Slack. I'm keeping the ZK version as 3.6 & adding the `accumulo-env.sh` as a post-script like you mentioned. I have updated the PR with these changes, please test and let me know. Thanks

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611111250
 
 
   > > It would have the same problems with 3.5.7. So, it should probably stay 3.4.14, or have some post-script to modify the class path in the Accumulo env script.
   > 
   > I discussed this with @milleruntime on Slack. I'm keeping the ZK version as 3.6 & adding the `accumulo-env.sh` as a post-script like you mentioned. I have updated the PR with these changes, please test and let me know. Thanks
   
   I like having the accumulo-env.sh as a post-script but I still think we should make the dockerfile work with 2.0.0 out of the box.  So this means the ZK veresion should be 3.4.14.  Sorry for misleading you.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-610979556
 
 
   While running the image I noticed the difference in the ZK tarball filename and corrected it & updated the PR. After this change, I tested the image and able to run ZK, Hadoop & Accumulo successfully.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611664160
 
 
   We should vote on a release for this, and then publish it to Dockerhub as described in the README (I think INFRA can help with that). If anyone is interested in pursuing this, please follow up on the dev mailing list.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] keith-turner commented on a change in pull request #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
keith-turner commented on a change in pull request #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#discussion_r406304659
 
 

 ##########
 File path: Dockerfile
 ##########
 @@ -73,11 +73,14 @@ RUN tar xzf hadoop.tar.gz -C /tmp/
 RUN tar xzf zookeeper.tar.gz -C /tmp/
 
 RUN mv /tmp/hadoop-$HADOOP_VERSION /opt/hadoop
-RUN mv /tmp/zookeeper-$ZOOKEEPER_VERSION /opt/zookeeper
+RUN mv /tmp/apache-zookeeper-$ZOOKEEPER_VERSION-bin /opt/zookeeper
 RUN mv /tmp/accumulo-$ACCUMULO_VERSION /opt/accumulo
 
 RUN /opt/accumulo/bin/accumulo-util build-native
 
+# The below line is required for Accumulo 2.0 to work with ZK 3.5 & above. 
 
 Review comment:
   ```suggestion
   # The below line is required for Accumulo 2.0 to work with ZK 3.5 & above.  This will not be needed for Accumulo 2.1
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
ctubbsii commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611153001
 
 
   > @karthick-rn I wonder if adding `ENV CLASSPATH /opt/zookeeper/lib/*` towards the end of the dockerfile instead of adding the accumulo-env.sh file would work. This should be picked up by Accumulo env, the only drawback is that we dont get control over the order of where `/opt/zookeeper/lib/*` is in the classpath.
   
   I like @keith-turner 's suggestion here. It's better than copying the env script in.
   
   @milleruntime I'm not following your use of "out-of-the-box" here. The way I see it, the Docker packaging is its own "box" (packaging). Accumulo 2.0.0 works fine, when properly configured, with ZK 3.5 or 3.6. So, it should be fine if the Docker packaging configures it that way. The only real difference is that you'll need to have the newer ZK running to use the Docker packaging of Accumulo, whereas if you were to package or deploy Accumulo manually, you could do so using a version as low as 3.4. But, I think it's fine for Docker to require newer.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] keith-turner merged pull request #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
keith-turner merged pull request #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] karthick-rn edited a comment on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
karthick-rn edited a comment on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611516281
 
 
   Looks like the order of the classpath is important here. When adding `ENV CLASSPATH /opt/zookeeper/lib/*` to Dockerfile, Accumulo positions the `ENV` directive first in the classpath like below. 
   <pre>
   <b>/opt/zookeeper/lib/*</b>:/opt/accumulo/conf:/opt/accumulo/lib/*:/opt/hadoop/etc/hadoop:/opt/zookeeper/*:/opt/hadoop/share/hadoop/client/* </pre>
   This has resulted in the below exception when trying to run queries on ashell.
   `Exception in thread "shell" java.lang.NoSuchMethodError: org.apache.commons.cli.Options.hasShortOption(Ljava/lang/String;)Z` 
   
   To maintain the order of the classpath, we tried several options & the one with `sed` works and doesn't require the post-script. I have updated the PR with the new changes, please review and let me know your thoughts/suggestions. Thanks
   
   
   
    
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [accumulo-docker] milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md

Posted by GitBox <gi...@apache.org>.
milleruntime commented on issue #12: Update Accumulo version in Dockerfile & README.md
URL: https://github.com/apache/accumulo-docker/pull/12#issuecomment-611040873
 
 
   @karthick-rn I am not sure how you are setting the classpath but here is what I am seeing:
   ```
   ~/workspace/accumulo-docker$ docker run accumulo classpath
   /opt/accumulo/conf:/opt/accumulo/lib/*:/opt/hadoop/etc/hadoop:/opt/zookeeper/*:/opt/hadoop/share/hadoop/client/*
   ```
   Then any other command throws the error:
   ```
   ~/workspace/accumulo-docker$ docker run accumulo --help
   2020-04-08 15:53:55,004 [start.Main] ERROR: Uncaught exception
   ...
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services