You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by spmallette <gi...@git.apache.org> on 2016/04/28 21:41:56 UTC
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
GitHub user spmallette opened a pull request:
https://github.com/apache/incubator-tinkerpop/pull/297
TINKERPOP-1268 Interactive and Executor Mode for Console
This PR covers:
https://issues.apache.org/jira/browse/TINKERPOP-1268
https://issues.apache.org/jira/browse/TINKERPOP-1157
https://issues.apache.org/jira/browse/TINKERPOP-1155
https://issues.apache.org/jira/browse/TINKERPOP-1156
I think that the console is working with a nice level of consistency now. We no longer have to warn people that the script they give as an init script is different than a they give to `-e` - it all executes in the same environment so no more discrepancies.
You can see some examples in the documentation for how `-e` and `-i` are used. The shell script simplified a bit - @dkuppitz if you see a better way to deal with the `getopts` thing please let me know. I need to track `-l` in the gremlin.sh because we use it to dynamically set some log4j stuff and other debug natured things. `getopts` kinda feels like overkill but - it's working so.....
I tested windows and it seemed to work.
Please give it a shot and see if you can break it. I tried to test as many combinations as I could think of but i may have fell short somewhere.
VOTE + 1
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP-1268
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tinkerpop/pull/297.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 #297
----
commit b1493b5b40434ff82c89790e3142819d569119ed
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T11:39:55Z
Deprecate ScriptExecutor.
commit b6853f052fc2efcf76f9d8e30bf95e6a22349fcc
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T17:46:02Z
Implemented more options for Gremlin Console startup.
Added -i for interactive mode and made -e execute in the Console rather than ScriptExecutor. Added switches to show/hide output and added a switch for "help". The existing method for -i still works as in "bin/gremlin.sh init.groovy"
commit 1ed7043721214b04ebb278ae45503fc893f2b960
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T18:01:53Z
Better exception messaging on failure of -e.
commit 3ce732a3e7113ea707dc39755c2fa2f06a5682b3
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T18:08:44Z
Updated changelog.
commit 05be605cd3df603772b15aad751310ea2d4b4b89
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T18:39:21Z
Allow override of verbosity to quiet when using -e.
Only override if the verbosity is not explicitly set.
commit 620f4787b3b6da01679e2f6b80aa3b1a9693f33a
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T18:59:37Z
Use -i as this is now the preferred way of starting interactive mode
commit 0e513cbc47f0885a0484b194ce81d31218dec6dd
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T19:00:04Z
Fixed up sections for console interactive/execute modes.
commit cd709ecb549d88d13dce0694c2d2d25de70baaf4
Author: Stephen Mallette <sp...@genoprime.com>
Date: 2016-04-28T19:02:17Z
Update upgrade docs for Console changes
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/297#discussion_r61685928
--- Diff: gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy ---
@@ -284,27 +315,98 @@ class Console {
return STANDARD_RESULT_PROMPT
}
- private void initializeShellWithScript(final String initScriptFile) {
+ private void executeInShell(final List<String> scriptAndArgs) {
+ final String scriptFile = scriptAndArgs[0]
try {
- final File file = new File(initScriptFile)
- file.eachLine { line ->
+ // check if this script comes with arguments. if so then set them up in an "args" bundle
+ if (scriptAndArgs.size() > 1) {
+ List<String> args = scriptAndArgs.subList(1, scriptAndArgs.size())
+ groovy.execute("args = [\"" + args.join('\",\"') + "\"]")
+ }
--- End diff --
Should declare `args` as either `null` or `[]` here if none are passed in, otherwise a script allowing 0 or more args would hit an error for `args` not declared.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/297#issuecomment-215569655
yeah - i thought about adding all the options to `getopts` and i did not like that idea especially since the console also supports long variants of each of the items:
```text
bin/gremlin.sh -e script.groovy
bin/gremlin.sh --execute=script.groovy
```
was getting too complicated to try to maintain that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/297#discussion_r61726988
--- Diff: gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy ---
@@ -284,27 +315,98 @@ class Console {
return STANDARD_RESULT_PROMPT
}
- private void initializeShellWithScript(final String initScriptFile) {
+ private void executeInShell(final List<String> scriptAndArgs) {
+ final String scriptFile = scriptAndArgs[0]
try {
- final File file = new File(initScriptFile)
- file.eachLine { line ->
+ // check if this script comes with arguments. if so then set them up in an "args" bundle
+ if (scriptAndArgs.size() > 1) {
+ List<String> args = scriptAndArgs.subList(1, scriptAndArgs.size())
+ groovy.execute("args = [\"" + args.join('\",\"') + "\"]")
+ }
--- End diff --
I'm suggesting this
```
if (scriptAndArgs.size() > 1) {
List<String> args = scriptAndArgs.subList(1, scriptAndArgs.size())
groovy.execute("args = [\"" + args.join('\",\"') + "\"]")
} else {
groovy.execute("args = []")
}
```
so that this input groovy script
```
println args
```
doesn't fail like this
```
/bin/gremlin.sh -e /tmp/foo.groovy
Error in /tmp/foo.groovy at [1: println args] - No such property: args for class: groovysh_evaluate
groovy.lang.MissingPropertyException: No such property: args for class: groovysh_evaluate
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/297#issuecomment-215548386
Looks good to me. I prefer this approach for option parsing:
https://github.com/apache/incubator-tinkerpop/blob/master/docker/scripts/build.sh#L40-L50
However, it requires that you handle all possible options and show a usage message if an option is unknown. That probably doesn't fit well in `gremlin.sh`, as the Java side is supposed to handle all "unknown" options and you likely don't want to keep the shell script and the Java side in sync whenever an option is added / removed.
VOTE: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/297#issuecomment-216036513
Looks good, Stephen. I reviewed the code, reviewed the docs, tested on Mac and Windows.
VOTE: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-tinkerpop/pull/297
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on a diff in the pull request:
https://github.com/apache/incubator-tinkerpop/pull/297#discussion_r61687353
--- Diff: gremlin-console/src/main/groovy/org/apache/tinkerpop/gremlin/console/Console.groovy ---
@@ -284,27 +315,98 @@ class Console {
return STANDARD_RESULT_PROMPT
}
- private void initializeShellWithScript(final String initScriptFile) {
+ private void executeInShell(final List<String> scriptAndArgs) {
+ final String scriptFile = scriptAndArgs[0]
try {
- final File file = new File(initScriptFile)
- file.eachLine { line ->
+ // check if this script comes with arguments. if so then set them up in an "args" bundle
+ if (scriptAndArgs.size() > 1) {
+ List<String> args = scriptAndArgs.subList(1, scriptAndArgs.size())
+ groovy.execute("args = [\"" + args.join('\",\"') + "\"]")
+ }
--- End diff --
That's done in [L166](https://github.com/apache/incubator-tinkerpop/pull/297/files#diff-94e9f2f26def3fb716d93349f670434aR166).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP-1268 Interactive and E...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/297#issuecomment-215588978
I mean to write this earlier....note that the addition of:
```xml
<dependency>
<groupId>commons-cli</groupId>
<artifactId>commons-cli</artifactId>
<version>1.2</version>
</dependency>
```
does affect the binary distribution for the console but it does not affect LICENSE/NOTICE. The NOTICE file included in commons-cli is boilerplate apache and therefore does not need to be included in our NOTICE.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---