Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/952#discussion_r222996609
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -216,14 +219,18 @@ class
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/947
Stephen "The Polyglot"
VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/929
I've created the tickets for the other languages on JIRA.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/929
Landed in 54ad5f45e73bae05b957b89539cb6a61d557f445, thanks @deejvince and
@beebs-systap !
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/929
I was waiting for +1 for another committer, as it's a new feature.
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/922#discussion_r219471784
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/client.js
---
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/922#discussion_r219469511
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/client.js
---
@@ -0,0 +1,87 @@
+/*
+ * Licensed to the Apache
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/929
Yes, as this is a small patch I'll apply it directly to `tp32`, instead of
asking the OO to rebase it.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/933
VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/915
It would be nice to include a changelog entry, as this contains some
breaking changes (i.e.: `IGremlinClient` member and `ResponseException`
constructor).
In any case, the changes in
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/929
lgtm, VOTE: +1
Once it gets approved, I'll create the tickets for the rest of the GLVs.
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/929#discussion_r217371869
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -49,12 +49,14 @@ class
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/929
Thanks for the patch @deejvince and thanks for filing TINKERPOP-2035
@beebs-systap !
I think its a good idea to expose ways to set the request header, as it's
part of the [pro
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
I think we are going deeper into the rabbit hole... looks like you are
focused on getting this one merged, if you think that's the best course of
action, I don't mind reviewing it ind
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
Max in-flight should be a threshold that, when reached, more connections
are created.
When max amount of connections is reached, we can either:
a) Continue queueing at connection
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
Yes, that's the design I was proposing, a minor note:
> Either returns a random `Connection` or returns the least-used
`Connection` based on an in-flight counter on the `Connecti
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
I think we should abandon the idea about removing connections from a pool
and the `ProxyConnection`, there is no point and it's not a good pattern.
All connections in the poo
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
> What I meant here is that the connection cannot be used again until
`SendAsync` completed. So, my suggestion for request pipelining is simply that
the connection is taken out of the pool
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
> Why do you want to keep the connections in the pool when a read/write
operation is executed on them? They can't be used at that time anyway and I
think that the current design is a lo
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
> Do you see other advantages?
It allows the client to prime the server with enough requests in order to
support maximum throughput. Considering the multithreaded asynchronous nature
of
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/922#discussion_r214825070
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/translator.js
---
@@ -0,0 +1,93 @@
+/*
+ * Licensed to the
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/922#discussion_r214823388
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -114,24 +114,26 @@ class
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/922#discussion_r214822151
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/client.js
---
@@ -0,0 +1,65 @@
+/*
+ * Licensed to the Apache
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/922
> I haven't considered the impact of this upon Bytecode so this may need to
be changed or addressed on the Bytecode side, or Bytecode may already handle
parameter maps?
The
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
Its one send at a time, that means that we have to send it one at a time
but not necessary awaiting to receive after each send.
```c#
// Send in a loop until the queue is exhausted
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/903#discussion_r214375950
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/AsyncAutoResetEvent.cs ---
@@ -0,0 +1,103 @@
+#region License
+
+/*
+ * Licensed to
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
Also, I'm not sure I understand how this will come into place with
TINKERPOP-1775.
Currently a connection is being used to one send and receive one
request/response pair at a tim
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/903#discussion_r214294630
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/AsyncAutoResetEvent.cs ---
@@ -0,0 +1,103 @@
+#region License
+
+/*
+ * Licensed to
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/903#discussion_r214292415
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs ---
@@ -33,91 +34,135 @@ internal class ConnectionPool : IDisposable
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/903#discussion_r214288173
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/AsyncAutoResetEvent.cs ---
@@ -0,0 +1,103 @@
+#region License
+
+/*
+ * Licensed to
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/903#discussion_r214288900
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/AsyncAutoResetEvent.cs ---
@@ -0,0 +1,103 @@
+#region License
+
+/*
+ * Licensed to
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/922
Glad to see you working on this @mattallenuk !
In line with what @spmallette was mentioning, I think we need expose a
similar API as the rest of GLVs. Python GLV is a good example
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/898
> I'd be happy to give the typings file generation
@mattallenuk I saw you've [created the
ticket](https://issues.apache.org/jira/browse/TINKERPOP-2027): Nice! we can
d
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
Merged ð! Thanks @mattallenuk for your contribution.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
I've pushed to the dev branch TINKERPOP-1977 with some minor fixes:
https://github.com/apache/tinkerpop/compare/tp32...TINKERPOP-1977
Travis started building it, if it succeeds
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
Its looking great!
I'll merge it to a dev branch into the project repo and fix the minor test
failure.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
At this stage you have to use "rebase interactive" to manually select your
commits and skip the rest.
If you have a tool to do that, it would make your job a lot easier.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
awesome!
First, lets restore `tp32` on your repo:
```
git fetch upstream tp32
git checkout tp32
git reset --hard upstream/tp32
git push origin tp32
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
its not a problem :)
As long as you make sure you don't loose your commits on the process (make
sure you push a backup branch to your personal repository).
I think the iss
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
I think the git branch is still based on`master` instead of `tp32`, that's
why github shows a lot of additional commits.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
Great contribution @mattallenuk!
We should merge it to `tp32`. @mattallenuk do you want to change the base
branch of this pull request and rebase? There shouldn't be any conflict
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/903#discussion_r211183634
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs ---
@@ -33,91 +34,134 @@ internal class ConnectionPool : IDisposable
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/915#discussion_r210506485
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/IResultSet.cs ---
@@ -0,0 +1,43 @@
+#region License
+/*
+ * Licensed to the Apache Software
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/903#discussion_r209870927
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ConnectionPool.cs ---
@@ -33,91 +34,134 @@ internal class ConnectionPool : IDisposable
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/842
Great work @danielcweber ! Thanks for your patience during the process.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
Nice! Looks great!
VOTE: +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/903
I'm back from OOO, I'll look at it this week.
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r209616766
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/sasl-authentication-tests.js
---
@@ -0,0 +1,68
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r209628120
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/sasl-authentication-tests.js
---
@@ -0,0 +1,68
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r209620249
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/auth/sasl-authenticator.js
---
@@ -0,0 +1,48
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/842
LGTM! VOTE: +1.
Thanks @danielcweber for the contribution!
Note to committers: I think it would be better to squash the commits ahead
of the merge.
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/842#discussion_r209605835
--- Diff: gremlin-dotnet/src/Gremlin.Net/Gremlin.Net.csproj ---
@@ -48,7 +48,7 @@ Please see the reference documentation at Apache
TinkerPop for more
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/842#discussion_r209598480
--- Diff: gremlin-dotnet/src/Gremlin.Net/Gremlin.Net.csproj ---
@@ -48,7 +48,7 @@ Please see the reference documentation at Apache
TinkerPop for more
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/898
Nice @daem0ndev! I think its a good idea to provide typing definitions
inside the package.
[Gremlin Variant
APIs](http://tinkerpop.apache.org/docs/current/tutorials/gremlin-language
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/890
Cherry picked and landed in c324002c7d2184b87ef1e90067c318ebf727311f
(`tp32`, `tp33` and `master`).
Thanks @arings!
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/890
`Pick` is covered by the cucumber tests and those features are passing:
https://github.com/apache/tinkerpop/blob/master/gremlin-javascript/src/main/javascript/gremlin-javascript/test/cucumber
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/890
Looks good, thanks @arings !
VOTE: +1
I've filed a ticket to keep track of this bug fix: TINKERPOP-2009.
Committers: note that we should merge it to `tp32`.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
We appreciate you taking the lead on this feature @mattallenuk . There is
no rush, we can leave the pull request open and wait for any progress.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/888
As this is a straightforward bug fix, I think its ok to do a CTR and avoid
waiting for an additional review/vote.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
`submit()` accepting `requestId` looks good!
About running the test locally, you should try (on root tinkerpop
directory):
- `mvn clean install -DskipTests` (run this once to
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/889
Looking at Python and C# GLVs support for authentication, I think there are
2 possible routes:
a) Provide a simple plain text auth mechanism, similar to Python and C#
GLVs:
https
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201601044
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -151,7 +156,16 @@ class
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201599552
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -151,7 +156,16 @@ class
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/879
BTW, regardless of the ongoing discussion, my VOTE: +1 :)
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201290758
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/remote-connection.js
---
@@ -33,9 +33,11 @@ class RemoteConnection
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201296778
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -100,24 +105,24 @@ class
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201290325
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -151,7 +156,16 @@ class
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201295616
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -151,7 +156,16 @@ class
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201296096
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -151,7 +156,16 @@ class
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r201289328
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -151,7 +156,16 @@ class
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/889#discussion_r200922477
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/authenticator.js
---
@@ -0,0 +1,32 @@
+/*
+ * Licensed to the
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/888
Thanks for changing the base branch @elliotttf
Looks good, VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/887
Thanks!
Please reopen this one against `tp32`, we will merge `tp32` to `tp33` to
`master` afterwards.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/887
I've filed a ticket to track this bug fix:
https://issues.apache.org/jira/browse/TINKERPOP-1995
When creating the ticket, I realized we should we fix this issue in all the
a
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/887
Nice catch! looks good: VOTE: +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/885
VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/880
lgtm, VOTE +1
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/867#discussion_r195645438
--- Diff: .travis.yml ---
@@ -20,9 +20,13 @@ install:
before_install:
- wget -q
https://packages.microsoft.com/config/ubuntu/14.04/packages
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/872
VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/873
Nice doc on the new method, VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/867
> Shouldn't the template be built and tested in the same phases as
Gremlin.Net itself?
Yes, I probably didn't make it clear :) I wanted to say any maven phase for
generating and building.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/867
I would like to propose that we generate and build `Gremlin.Net.Template`
project and run its tests as part of the `package` phase but we require nuget
(to run `nuget pack`) only as part of the
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/867#discussion_r193065380
--- Diff: gremlin-dotnet/src/pom.xml ---
@@ -89,6 +93,54 @@ limitations under the License.
false
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/802
Nice!
VOTE +1
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/868#discussion_r191665300
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/Connection.cs ---
@@ -114,11 +116,18 @@ internal class Connection : IConnection
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/868#discussion_r191660636
--- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/ResultSet.cs ---
@@ -0,0 +1,99 @@
+#region License
+
+/*
+ * Licensed to the Apache
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/866
VOTE: +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/863
Its nice to have it as a explicit graph feature, VOTE +1.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/842
Sorry I'm late to the party, I haven't got much time during the past weeks:
nice contribution @danielcweber !
I'm a little concerned about the difference between
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/869
Nice!
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/867
Maybe we could wait until `dotnet` toolchain supports packaging and pushing
templates (is there a ticket for it?), instead of increasing the complexity of
the deploy process by requiring Mono.
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/859
lgtm, VOTE +1
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/857#discussion_r184057436
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/StandardVerificationStrategy.java
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/858#discussion_r184052492
--- Diff:
gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js
---
@@ -33,7 +33,7 @@ const
GitHub user jorgebay opened a pull request:
https://github.com/apache/tinkerpop/pull/858
TINKERPOP-1943 Support GraphSON3 in Gremlin-JavaScript
Add support for GraphSON3 to the JavaScript GLV.
https://issues.apache.org/jira/browse/TINKERPOP-1943
All gherkin
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/855
VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/843
VOTE +1
---
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/853
VOTE +1
---
Github user jorgebay commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/855#discussion_r183959745
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
---
@@ -63,6 +63,8 @@
*/
public
Github user jorgebay commented on the issue:
https://github.com/apache/tinkerpop/pull/851
Indeed, VOTE +1 :)
---
1 - 100 of 282 matches
Mail list logo
