Details
-
Improvement
-
Status: Closed
-
Minor
-
Resolution: Fixed
-
3.0.1-incubating
-
None
Description
If :install tanks:
gremlin> :install org.apache.tinkerpop hadoop-gremlin 3.1.0-SNAPSHOT ==>Error grabbing Grapes -- [download failed: io.netty#netty;3.6.2.Final!netty.jar(bundle)]
try to clean up the directory that gets created or else future attempts do this:
gremlin> :install org.apache.tinkerpop hadoop-gremlin 3.1.0-SNAPSHOT ==>a module with the name hadoop-gremlin is already installed
This is also a problem for bin/gremlin-server.sh -i
Attachments
Activity
I ended up testing this manually, using Titan as the example.
Prep
rm ~/.groovy/grapeConfig.xml rm -rf ~/.groovy/grapes/org.apache.giraph/ rm -rf ~/.m2/repository/org/apache/giraph/
Gremlin Server
cd gremlin-server/target/apache-gremlin-server-3.0.2-SNAPSHOT-standalone/
- Attempt to install titan-all should fail
./bin/gremlin-server.sh -i com.thinkaurelius.titan titan-all 1.0.0
- Verify that plugin directory titan-all doesn't exist
ls ./ext/titan-all
- Attempt to install existing plugin tinkergraph-gremlin should fail
./bin/gremlin-server.sh -i org.apache.tinkerpop tinkergraph-gremlin 3.0.2-SNAPSHOT
- Verify that plugin directory should still exist
ls ./ext/tinkergraph-gremlin
Gremlin Console
cd gremlin-console/target/apache-gremlin-console-3.0.2-SNAPSHOT-standalone/
- Attempt to install titan-all should fail
./bin/gremlin.sh :install com.thinkaurelius.titan titan-all 1.0.0 :q
- Verify that plugin directory titan-all doesn't exist
ls ./ext/titan-all
- Attempt to install existing plugin tinkergraph-gremlin should fail
./bin/gremlin.sh :install org.apache.tinkerpop tinkergraph-gremlin 3.0.2-SNAPSHOT :q
- Verify that plugin directory should still exist
ls ./ext/tinkergraph-gremlin
GitHub user pluradj opened a pull request:
https://github.com/apache/incubator-tinkerpop/pull/110
TINKERPOP3-858 Cleanup after failed plugin install
https://issues.apache.org/jira/browse/TINKERPOP3-858 see manual testing steps in the JIRA
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-858
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tinkerpop/pull/110.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 #110
commit 955a98f7af541cc10674defeca53c4870ca8f441
Author: Jason Plurad <pluradj@apache.org>
Date: 2015-10-16T14:27:51Z
TINKERPOP3-858 Cleanup after failed plugin install
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#issuecomment-148755126
VOTE: +1
Github user mhfrantz commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#discussion_r42271237
— Diff: gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy —
@@ -53,6 +53,12 @@ class InstallCommand extends CommandSupport
catch (Exception ex) {
+ if (!(ex instanceof IllegalStateException)) {
+ // IllegalStateException is thrown if a module with the same name is already installed.
+ final def uninstall = new UninstallCommand(shell, mediator)
+ final List<String> module = Collections.singletonList(artifact.getArtifact())
+ uninstall.execute(module)
— End diff –
Can uninstall throw an exception?
Github user pluradj commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#issuecomment-148801984
@dkuppitz `DependencyGrabber.copyDependenciesToPath()` throws two exceptions explicitly, `IOException` and `IllegalStateException`. `RuntimeException` occurs if there is an error grabbing grapes. This is the initial bug report, and its failed directory should be rolled back.
`IOException` occurs if there are problems creating the directories under `ext`. Rollback should occur here, but most likely there isn't even a directory to delete.
`IllegalStateException` occurs if plugin module already exists. For example, `tinkergraph-gremlin` is installed by default out of the box. If the code doesn't catch the `IllegalStateException`, `tinkergraph-gremlin` would get uninstalled if the user tried to install it a second time.
Github user pluradj commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#discussion_r42276002
— Diff: gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy —
@@ -53,6 +53,12 @@ class InstallCommand extends CommandSupport
catch (Exception ex) {
+ if (!(ex instanceof IllegalStateException)) {
+ // IllegalStateException is thrown if a module with the same name is already installed.
+ final def uninstall = new UninstallCommand(shell, mediator)
+ final List<String> module = Collections.singletonList(artifact.getArtifact())
+ uninstall.execute(module)
— End diff –
No, I didn't see any exceptions declared or thrown in the class.
Github user dkuppitz commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#issuecomment-148830986
+1
Github user okram commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#issuecomment-148851150
If @pluradj votes +1, then there will be three. Thus, @pluradj – if you think your work is solid, then +1 it and tally the vote and merge. Good luck.
Github user pluradj commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#issuecomment-148955829
Thanks for the comments. I've centralized the rollback logic into DependencyGrabber, and added a couple unit tests.
Github user pluradj commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#issuecomment-148955862
Vote: +1
Github user okram commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/110#issuecomment-148958687
You have 3 +1 votes. Please merge and if this is going into tp30, don't forget to merge to master as well. Good luck.
pluradj said he was going to take a look at this one. Here's some pointers to ease your work. For the console, I believe some additional logic here to "clean up" should solve the problem:
https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/commands/InstallCommand.groovy#L56
for the server, similar logic would need to be injected here:
https://github.com/apache/incubator-tinkerpop/blob/master/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinServerInstall.java#L40
I think it's that simple - hopefully it's not a bigger deal than that. Not sure that you can easily write tests for this stuff, but if you can come up with something that would be nice.