[GitHub] [tinkerpop] humbleengineer commented on pull request #1293: TINKERPOP-2264 update gremlin-python tests to be timezone/system-independent, related minor Docker changes
humbleengineer commented on pull request #1293: URL: https://github.com/apache/tinkerpop/pull/1293#issuecomment-642154171 > Do you mind squashing the "TINKERPOP-2264" to one commit and all your "docker" related ones to another? > > Other than that, this looks good - VOTE +1 Done, thank you for the feedback. 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: us...@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1293: TINKERPOP-2264 update gremlin-python tests to be timezone/system-independent, related minor Docker changes
spmallette commented on pull request #1293: URL: https://github.com/apache/tinkerpop/pull/1293#issuecomment-642110333 Do you mind squashing the "TINKERPOP-2264" to one commit and all your "docker" related ones to another? Other than that, this looks good - VOTE +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: us...@infra.apache.org
[GitHub] [tinkerpop] jorgebay opened a new pull request #1294: TINKERPOP-2377 .NET GLV: Check for null earlier
jorgebay opened a new pull request #1294: URL: https://github.com/apache/tinkerpop/pull/1294 https://issues.apache.org/jira/browse/TINKERPOP-2377 I think this addresses the issue. 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: us...@infra.apache.org
[tinkerpop] 01/01: TINKERPOP-2377 .NET GLV: Check for null earlier
This is an automated email from the ASF dual-hosted git repository.
jorgebg pushed a commit to branch TINKERPOP-2377
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit c7a552739f12102a32689e6828b215871d29f010
Author: Jorge Bay Gondra
AuthorDate: Wed Jun 10 15:49:19 2020 +0200
TINKERPOP-2377 .NET GLV: Check for null earlier
---
gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs | 12 ++--
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs
b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs
index 490e4e4..5db0fe8 100644
--- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs
+++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/Bytecode.cs
@@ -110,17 +110,17 @@ namespace Gremlin.Net.Process.Traversal
private object ConvertArgument(object argument, bool searchBindings)
{
+if (null == argument)
+{
+return null;
+}
+
if (searchBindings)
{
var variable = Bindings.GetBoundVariable(argument);
if (variable != null)
return new Binding(variable, ConvertArgument(argument,
false));
}
-
-if (null == argument)
-{
-return null;
-}
if (IsDictionaryType(argument.GetType()))
{
@@ -164,4 +164,4 @@ namespace Gremlin.Net.Process.Traversal
return type.IsConstructedGenericType &&
type.GetGenericTypeDefinition() == typeof(HashSet<>);
}
}
-}
\ No newline at end of file
+}
[tinkerpop] branch TINKERPOP-2377 created (now c7a5527)
This is an automated email from the ASF dual-hosted git repository. jorgebg pushed a change to branch TINKERPOP-2377 in repository https://gitbox.apache.org/repos/asf/tinkerpop.git. at c7a5527 TINKERPOP-2377 .NET GLV: Check for null earlier This branch includes the following new commits: new c7a5527 TINKERPOP-2377 .NET GLV: Check for null earlier The 1 revisions listed above as "new" are entirely new to this repository and will be described in separate emails. The revisions listed as "add" were already present in the repository and have only been added to this reference.
[GitHub] [tinkerpop] spmallette commented on a change in pull request #1293: TINKERPOP-2264 update gremlin-python tests to be timezone/system-independent, related minor Docker changes
spmallette commented on a change in pull request #1293:
URL: https://github.com/apache/tinkerpop/pull/1293#discussion_r438109277
##
File path: docker/gremlin-server.sh
##
@@ -57,7 +57,7 @@ echo "Using Gremlin Server $GREMLIN_SERVER_VERSION"
sed -e "s/GREMLIN_SERVER_VERSION\$/${GREMLIN_SERVER_VERSION}/"
docker/gremlin-server/Dockerfile.template > Dockerfile
docker build -t tinkerpop:${BUILD_TAG} .
-docker run ${TINKERPOP_TEST_DOCKER_OPTS} ${REMOVE_CONTAINER} -ti -v
"${HOME}"/.groovy:/root/.groovy -v "${HOME}"/.m2:/root/.m2
tinkerpop:${BUILD_TAG} ${@}
+docker run ${TINKERPOP_TEST_DOCKER_OPTS} ${REMOVE_CONTAINER} -ti -p
45940:45940 -p 45941:45941 -v "${HOME}"/.groovy:/root/.groovy -v
"${HOME}"/.m2:/root/.m2 tinkerpop:${BUILD_TAG} ${@}
Review comment:
ahhh - so maybe no one has been running on mac until now.
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:
us...@infra.apache.org
[GitHub] [tinkerpop] humbleengineer commented on a change in pull request #1293: TINKERPOP-2264 update gremlin-python tests to be timezone/system-independent, related minor Docker changes
humbleengineer commented on a change in pull request #1293:
URL: https://github.com/apache/tinkerpop/pull/1293#discussion_r438074087
##
File path: docker/gremlin-server.sh
##
@@ -57,7 +57,7 @@ echo "Using Gremlin Server $GREMLIN_SERVER_VERSION"
sed -e "s/GREMLIN_SERVER_VERSION\$/${GREMLIN_SERVER_VERSION}/"
docker/gremlin-server/Dockerfile.template > Dockerfile
docker build -t tinkerpop:${BUILD_TAG} .
-docker run ${TINKERPOP_TEST_DOCKER_OPTS} ${REMOVE_CONTAINER} -ti -v
"${HOME}"/.groovy:/root/.groovy -v "${HOME}"/.m2:/root/.m2
tinkerpop:${BUILD_TAG} ${@}
+docker run ${TINKERPOP_TEST_DOCKER_OPTS} ${REMOVE_CONTAINER} -ti -p
45940:45940 -p 45941:45941 -v "${HOME}"/.groovy:/root/.groovy -v
"${HOME}"/.m2:/root/.m2 tinkerpop:${BUILD_TAG} ${@}
Review comment:
I suspect it's because [Docker on Mac runs in its own
VM](https://docs.docker.com/docker-for-mac/docker-toolbox/#the-docker-desktop-on-mac-environment),
instead of in a container directly on the (Linux) host, so there's more to it
than just EXPOSE commands in the Dockerfile.
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:
us...@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a change in pull request #1293: TINKERPOP-2264 update gremlin-python tests to be timezone/system-independent, related minor Docker changes
spmallette commented on a change in pull request #1293:
URL: https://github.com/apache/tinkerpop/pull/1293#discussion_r438046045
##
File path: docker/gremlin-server.sh
##
@@ -57,7 +57,7 @@ echo "Using Gremlin Server $GREMLIN_SERVER_VERSION"
sed -e "s/GREMLIN_SERVER_VERSION\$/${GREMLIN_SERVER_VERSION}/"
docker/gremlin-server/Dockerfile.template > Dockerfile
docker build -t tinkerpop:${BUILD_TAG} .
-docker run ${TINKERPOP_TEST_DOCKER_OPTS} ${REMOVE_CONTAINER} -ti -v
"${HOME}"/.groovy:/root/.groovy -v "${HOME}"/.m2:/root/.m2
tinkerpop:${BUILD_TAG} ${@}
+docker run ${TINKERPOP_TEST_DOCKER_OPTS} ${REMOVE_CONTAINER} -ti -p
45940:45940 -p 45941:45941 -v "${HOME}"/.groovy:/root/.groovy -v
"${HOME}"/.m2:/root/.m2 tinkerpop:${BUILD_TAG} ${@}
Review comment:
I'm still not smart enough about docker to know why this worked all this
time without the `-p` - @robertdale or @dkuppitz any ideas?
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:
us...@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a change in pull request #1293: TINKERPOP-2264 update gremlin-python tests to be timezone/system-independent, related minor Docker changes
spmallette commented on a change in pull request #1293: URL: https://github.com/apache/tinkerpop/pull/1293#discussion_r438045632 ## File path: gremlin-python/pom.xml ## @@ -311,7 +311,7 @@ limitations under the License. -
[GitHub] [tinkerpop] humbleengineer opened a new pull request #1293: TINKERPOP-2264 update gremlin-python tests to be timezone/system-independent, related minor Docker changes
humbleengineer opened a new pull request #1293: URL: https://github.com/apache/tinkerpop/pull/1293 The datetime conversion tests didn't pass when running on a Mac in London, so they've been updated to use `calendar.timegm` just like the code under test was in the [original PR for TINKERPOP-2264](https://github.com/apache/tinkerpop/pull/1165). Also, in order to run & debug the integration tests I updated gremlin-python's pom.xml to refer to a `python` executable rather than `python2`. Since [Docker on Mac runs in its own VM](https://docs.docker.com/docker-for-mac/docker-toolbox/#the-docker-desktop-on-mac-environment) I updated the integration test `gremlin-server.sh` to explicitly publish its ports using `-p`. Happy to revert these if they're too environment-specific. `gremlin-python` install target builds clean using IntelliJ Maven support (I get a javadoc-related error building from command 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: us...@infra.apache.org
