[jira] [Updated] (TINKERPOP-2003) After a long period (almost 5days)of parallel requests(almost 200 request per second),client.submit blocked
[ https://issues.apache.org/jira/browse/TINKERPOP-2003?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] xifenghz updated TINKERPOP-2003: Description: After a long period (almost 5days)of parallel requests(almost 200 request per second),client.submit blocked for scriptEvaluationTimeout configured,and return timeout.At the same time,many TCP connections in Gremlin Server are in CLOSE_WAIT state. Here is my usages: cluster = Cluster.build(FileUtils.toFile(config)).create(); client = cluster.connect(); ResultSet resultSet = null; {color:#ff}synchronized (Service.class){color} { resultSet = client.submit(gremlinScript); } CompletableFuture> future = null; List results = null; future = resultSet.all(); results = future.get(); The code snippet above runs in a java web app based on SpringMVC.I found a PR related to my question. [https://github.com/apache/tinkerpop/pull/367/commits/58d8bade7425c7a7865382990eaaed2b7d90659c#diff-06d5dfb02d5d2c807c2387c3e9338709] In this PR,the author seems to say gremlin doer not support high concurrency。 {color:#ff}Removed recommendations for submitting parallel requests on a session from docs.{color} {color:#33}So,I submit the gremlin script in order(use synchronized keyword,just as my code snippet ).But in order to lift throughput,I do not lock the code for getting result from Gremlin Server。I want to know some details about how Gremlin Server receive and process requests,such as the receiver and executor threads are the same one?If so ,the Gremlin server can only process the requests one by one ? {color} {color:#33}I wonder whether or not Gremlin supports high concurrency(such as 200 second per second,including simple vertex or edge queries and complex combinatorial queries).If so,please help to find out why the process time for a simple script such as query vertex exceed scriptEvaluationTimeout. If not,is there other ways to make Gremlin Server support high concurrency,such as multi-gremlinserver。{color} Thank you sincerely for your help。 was: After a long period (almost 5days)of parallel requests(almost 200 request per second),client.submit blocked for scriptEvaluationTimeout configured,and return timeout.At the same time,many TCP connections in Gremlin Server are in CLOSE_WAIT state. Here is my usages: cluster = Cluster.build(FileUtils.toFile(config)).create(); client = cluster.connect(); ResultSet resultSet = null; {color:#FF}synchronized (Service.class){color} { resultSet = client.submit(gremlinScript); } CompletableFuture> future = null; List results = null; future = resultSet.all(); results = future.get(); The code snippet above runs in a java web app based on SpringMVC.I found a PR related to my question. [https://github.com/apache/tinkerpop/pull/367/commits/58d8bade7425c7a7865382990eaaed2b7d90659c#diff-06d5dfb02d5d2c807c2387c3e9338709] In this PR,the author seems to say gremlin doer not support high concurrency。 {color:#FF}Removed recommendations for submitting parallel requests on a session from docs.{color} {color:#33}So,I submit the gremlin script in order(use synchronized keyword,just as my code snippet ).But in order to lift throughput,I do not lock the code for getting result from Gremlin Server。{color} {color:#33}I don't know whether or not Gremlin supports high concurrency(such as 200 second per second,including simple vertex or edge queries and complex combinatorial queries)。If so,please help to find out why the process time for a simple script such as query vertex exceed scriptEvaluationTimeout. If not,is there other ways to make Gremlin Server support high concurrency,such as multi-gremlinserver。{color} Thank you sincerely for your help。 > After a long period (almost 5days)of parallel requests(almost 200 request per > second),client.submit blocked > --- > > Key: TINKERPOP-2003 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2003 > Project: TinkerPop > Issue Type: Bug > Components: driver >Affects Versions: 3.3.0 >Reporter: xifenghz >Priority: Major > > After a long period (almost 5days)of parallel requests(almost 200 request per > second),client.submit blocked for scriptEvaluationTimeout configured,and > return timeout.At the same time,many TCP connections in Gremlin Server are in > CLOSE_WAIT state. > Here is my usages: > cluster = Cluster.build(FileUtils.toFile(config)).create(); > client = cluster.connect(); > ResultSet resultSet = null; > {color:#ff}synchronized (Service.class){color} > { resultSet = client.submit(gremlinScript); } > CompletableFuture> future = null; > List results = null; > future = resultSet.all(); > results = future.get(); > > The code snippet above ru
[jira] [Commented] (TINKERPOP-1990) Add a shortestPath() step
[ https://issues.apache.org/jira/browse/TINKERPOP-1990?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16539457#comment-16539457 ] ASF GitHub Bot commented on TINKERPOP-1990: --- Github user twilmes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/882#discussion_r201547477 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/search/path/ShortestPathVertexProgram.java --- @@ -0,0 +1,557 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.process.computer.search.path; + +import org.apache.commons.configuration.Configuration; +import org.apache.tinkerpop.gremlin.process.computer.*; --- End diff -- Looks like the IDE wild-carded a few of these imports > Add a shortestPath() step > - > > Key: TINKERPOP-1990 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1990 > Project: TinkerPop > Issue Type: Improvement > Components: process >Reporter: Daniel Kuppitz >Assignee: Daniel Kuppitz >Priority: Major > > Implement {{ShortestPathVertexProgram}} and a {{shortestPath()}} step. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] tinkerpop pull request #882: TINKERPOP-1990 Add a shortestPath() step
Github user twilmes commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/882#discussion_r201547477 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/search/path/ShortestPathVertexProgram.java --- @@ -0,0 +1,557 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.process.computer.search.path; + +import org.apache.commons.configuration.Configuration; +import org.apache.tinkerpop.gremlin.process.computer.*; --- End diff -- Looks like the IDE wild-carded a few of these imports ---
[jira] [Created] (TINKERPOP-2003) After a long period (almost 5days)of parallel requests(almost 200 request per second),client.submit blocked
xifenghz created TINKERPOP-2003: --- Summary: After a long period (almost 5days)of parallel requests(almost 200 request per second),client.submit blocked Key: TINKERPOP-2003 URL: https://issues.apache.org/jira/browse/TINKERPOP-2003 Project: TinkerPop Issue Type: Bug Components: driver Affects Versions: 3.3.0 Reporter: xifenghz After a long period (almost 5days)of parallel requests(almost 200 request per second),client.submit blocked for scriptEvaluationTimeout configured,and return timeout.At the same time,many TCP connections in Gremlin Server are in CLOSE_WAIT state. Here is my usages: cluster = Cluster.build(FileUtils.toFile(config)).create(); client = cluster.connect(); ResultSet resultSet = null; {color:#FF}synchronized (Service.class){color} { resultSet = client.submit(gremlinScript); } CompletableFuture> future = null; List results = null; future = resultSet.all(); results = future.get(); The code snippet above runs in a java web app based on SpringMVC.I found a PR related to my question. [https://github.com/apache/tinkerpop/pull/367/commits/58d8bade7425c7a7865382990eaaed2b7d90659c#diff-06d5dfb02d5d2c807c2387c3e9338709] In this PR,the author seems to say gremlin doer not support high concurrency。 {color:#FF}Removed recommendations for submitting parallel requests on a session from docs.{color} {color:#33}So,I submit the gremlin script in order(use synchronized keyword,just as my code snippet ).But in order to lift throughput,I do not lock the code for getting result from Gremlin Server。{color} {color:#33}I don't know whether or not Gremlin supports high concurrency(such as 200 second per second,including simple vertex or edge queries and complex combinatorial queries)。If so,please help to find out why the process time for a simple script such as query vertex exceed scriptEvaluationTimeout. If not,is there other ways to make Gremlin Server support high concurrency,such as multi-gremlinserver。{color} Thank you sincerely for your help。 -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Updated] (TINKERPOP-1998) IoGraphTest use different schemas for standard and readGraph configurations
[ https://issues.apache.org/jira/browse/TINKERPOP-1998?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] stephen mallette updated TINKERPOP-1998: Component/s: test-suite > IoGraphTest use different schemas for standard and readGraph configurations > --- > > Key: TINKERPOP-1998 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1998 > Project: TinkerPop > Issue Type: Improvement > Components: test-suite >Affects Versions: 3.3.0 >Reporter: Horacio Hoyos Rodriguez >Priority: Minor > > In ArangoDB labels used by vertices and edges must be known in order to > create a graph. As a result this information should be available at startup, > i.e via configuration. For example _shouldReadWriteModernToFileWithHelpers_ > test expects a different schema from the *standard* graph than from the > *readGraph*, the latter attempts to create vertices in a (default) vertex > collection instead of using the people or software ones that were used for > the former. > It would also help, for graph technologies that require schemas, to have a > document that describe the schemas used/expected in the tests. I am > collecting this information atm, but having to run failing tests to capture > what label/edges I am missing for each test is a PITA. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
Re: [DISCUSS] Review Process
Hi, I feel like the project has become a bit too big and dispersed. A large portion of the emails, jira or otherwise are irrelevant to my interest/time/work. Perhaps for version 4, TinkerPop could be broken up into more focused projects with their own jira/email/process management. gremlin-language gremlin-server js-driver python-driver java-driver .net-driver reference implementation ... Thanks Pieter Perhaps for version 4 the project should be broken up On 10/07/2018 22:01, Jason Plurad wrote: Thanks for starting this conversation, Stephen. Lots of interesting tidbits here, and perhaps some we can apply to other OSS projects. I'm not sure if committers/PMC members have just not had time to do reviews or have not felt comfortable doing them Probably a combination of both, especially with the GLVs. I personally chase votes in the background to get PRs to merge.and, I don't want to do that anymore. Amazing that you did that, but I agree that nagging is not a great path forward. it is perfectly fine to review/VOTE in the following manner (as examples) It'd be great to have these examples added to the maintainer guidelines. When I do code reviews, sometimes I feel like one-liner votes are a bit of a cop out, but having examples like this would lower the mental hurdle to getting started on reviewing. It would also be nice for non-committers to do reviews - i don't know how to encourage that behavior though. I agree on this, and it would be particularly important on areas of the code where we only have one primary committer, like each GLV. If we come to agreement on a new policy, I'd suggest that if we get the docs written up and published, then we can mention it on gremlin-users sort of as a heads up to those interested in getting more involved. Their participation helps drive out releases, and new releases attract more users. Regarding the proposal, a single binding +1 from a committer with a 1 week lazy consensus sounds fine to me. If the contribution is a major feature or significant change, the expectation is that the committer realizes this and holds it open for 3 votes before committing. On Tue, Jul 10, 2018 at 1:46 PM, Stephen Mallette wrote: Good point, Ted - that wasn't clear and in truth I didn't think that through well. I think we could say that that the +1 would come from a committer. If the committer and submitter are one in the same then it has its single VOTE and technically, the PR just goes through the week long cooling period and could be merged after that. In the event the PR submitter is not a committer then they would require at least one committer to be on board with a +1 and a week long wait. Ideally, I think we can trust committers enough to do smart things with this change in process. I would hope that a committer who submits a PR that is especially complex and touches scary code parts or breaks user/provider APIs requests an actual review from another committer who might be able to spot some problems even if the 1 week cool down passes by. I don't want to subvert the good things that reviews do, but I don't want them holding up simple code changes either. I'd really like it if we introduced this change and we still got multiple +1s on PRs. It would also be nice for non-committers to do reviews - i don't know how to encourage that behavior though. On Tue, Jul 10, 2018 at 1:26 PM Ted Wilmes wrote: I fell way off the PR review train, I'll get back on. For clarification, is that a +1 on top of the submitter +1? I'm thinking you all just meant the submitter's +1 would be adequate after the lazy consensus period but wanted to be sure. I'd be fine to moving with that. My impression is that with the folks involved, if a submitter feels that another set of eyes is really required and lazy consensus is not adequate, regardless of the policy, that review will be sought and performed prior to merge. --Ted On Tue, Jul 10, 2018 at 11:44 AM Stephen Mallette wrote: It looks like its disabled for this project. I don't think we can use the GitHub integration without getting off our Apache Mirror (which we've discussed, but not really pulled the trigger on for no particular reason other than the hassle of changing everything). Does it have to be in that order? I was thinking that the as long as there is a single +1 at any time in that week (or after that week) then it would be good to merge On Tue, Jul 10, 2018 at 12:36 PM Robert Dale wrote: There might be a better alternative to privately nagging ;-) Github has a feature on the sidebar that can be used to request reviews from individuals or groups. The heading has 'Reviewers' and, when it's active, has a gear icon to select people. Github will then email the reviewers with the request. It looks like its disabled for this project. I like the idea of adding the option of having a single vote and a week to soak. Does it have to be in that order? Or can the vot
Re: [DISCUSS] Review Process
Thanks for starting this conversation, Stephen. Lots of interesting tidbits here, and perhaps some we can apply to other OSS projects. > I'm not sure if committers/PMC members have just not had time to do reviews or have not felt comfortable doing them Probably a combination of both, especially with the GLVs. > I personally chase votes in the background to get PRs to merge.and, I don't want to do that anymore. Amazing that you did that, but I agree that nagging is not a great path forward. > it is perfectly fine to review/VOTE in the following manner (as examples) It'd be great to have these examples added to the maintainer guidelines. When I do code reviews, sometimes I feel like one-liner votes are a bit of a cop out, but having examples like this would lower the mental hurdle to getting started on reviewing. > It would also be nice for non-committers to do reviews - i don't know how to encourage that behavior though. I agree on this, and it would be particularly important on areas of the code where we only have one primary committer, like each GLV. If we come to agreement on a new policy, I'd suggest that if we get the docs written up and published, then we can mention it on gremlin-users sort of as a heads up to those interested in getting more involved. Their participation helps drive out releases, and new releases attract more users. Regarding the proposal, a single binding +1 from a committer with a 1 week lazy consensus sounds fine to me. If the contribution is a major feature or significant change, the expectation is that the committer realizes this and holds it open for 3 votes before committing. On Tue, Jul 10, 2018 at 1:46 PM, Stephen Mallette wrote: > Good point, Ted - that wasn't clear and in truth I didn't think that > through well. I think we could say that that the +1 would come from a > committer. If the committer and submitter are one in the same then it has > its single VOTE and technically, the PR just goes through the week long > cooling period and could be merged after that. In the event the PR > submitter is not a committer then they would require at least one committer > to be on board with a +1 and a week long wait. > > Ideally, I think we can trust committers enough to do smart things with > this change in process. I would hope that a committer who submits a PR that > is especially complex and touches scary code parts or breaks user/provider > APIs requests an actual review from another committer who might be able to > spot some problems even if the 1 week cool down passes by. I don't want to > subvert the good things that reviews do, but I don't want them holding up > simple code changes either. I'd really like it if we introduced this change > and we still got multiple +1s on PRs. It would also be nice for > non-committers to do reviews - i don't know how to encourage that behavior > though. > > > > > On Tue, Jul 10, 2018 at 1:26 PM Ted Wilmes wrote: > > > I fell way off the PR review train, I'll get back on. For clarification, > is > > that a +1 on top of the submitter +1? I'm thinking you > > all just meant the submitter's +1 would be adequate after the lazy > > consensus period but wanted to be sure. I'd be fine to moving with that. > My > > impression is that with the folks involved, if a submitter feels that > > another set of eyes is really required and lazy consensus is not > adequate, > > regardless of the policy, that review will be sought and performed prior > to > > merge. > > > > --Ted > > > > On Tue, Jul 10, 2018 at 11:44 AM Stephen Mallette > > wrote: > > > > > > It looks like its disabled for this project. > > > > > > I don't think we can use the GitHub integration without getting off our > > > Apache Mirror (which we've discussed, but not really pulled the trigger > > on > > > for no particular reason other than the hassle of changing everything). > > > > > > > Does it have to be in that order? > > > > > > I was thinking that the as long as there is a single +1 at any time in > > that > > > week (or after that week) then it would be good to merge > > > > > > On Tue, Jul 10, 2018 at 12:36 PM Robert Dale > wrote: > > > > > > > There might be a better alternative to privately nagging ;-) Github > > has > > > a > > > > feature on the sidebar that can be used to request reviews from > > > individuals > > > > or groups. The heading has 'Reviewers' and, when it's active, has a > > gear > > > > icon to select people. Github will then email the reviewers with the > > > > request. It looks like its disabled for this project. > > > > > > > > I like the idea of adding the option of having a single vote and a > week > > > to > > > > soak. Does it have to be in that order? Or can the vote take place > > any > > > > time during that week? Otherwise, you could end up with no one > voting > > in > > > > the first week, get a vote, then waiting another week. > > > > > > > > Robert Dale > > > > > > > > > > > > On Tue, Jul 10, 2018 at 7:22 AM Jorge Ba
Re: [DISCUSS] Review Process
Good point, Ted - that wasn't clear and in truth I didn't think that through well. I think we could say that that the +1 would come from a committer. If the committer and submitter are one in the same then it has its single VOTE and technically, the PR just goes through the week long cooling period and could be merged after that. In the event the PR submitter is not a committer then they would require at least one committer to be on board with a +1 and a week long wait. Ideally, I think we can trust committers enough to do smart things with this change in process. I would hope that a committer who submits a PR that is especially complex and touches scary code parts or breaks user/provider APIs requests an actual review from another committer who might be able to spot some problems even if the 1 week cool down passes by. I don't want to subvert the good things that reviews do, but I don't want them holding up simple code changes either. I'd really like it if we introduced this change and we still got multiple +1s on PRs. It would also be nice for non-committers to do reviews - i don't know how to encourage that behavior though. On Tue, Jul 10, 2018 at 1:26 PM Ted Wilmes wrote: > I fell way off the PR review train, I'll get back on. For clarification, is > that a +1 on top of the submitter +1? I'm thinking you > all just meant the submitter's +1 would be adequate after the lazy > consensus period but wanted to be sure. I'd be fine to moving with that. My > impression is that with the folks involved, if a submitter feels that > another set of eyes is really required and lazy consensus is not adequate, > regardless of the policy, that review will be sought and performed prior to > merge. > > --Ted > > On Tue, Jul 10, 2018 at 11:44 AM Stephen Mallette > wrote: > > > > It looks like its disabled for this project. > > > > I don't think we can use the GitHub integration without getting off our > > Apache Mirror (which we've discussed, but not really pulled the trigger > on > > for no particular reason other than the hassle of changing everything). > > > > > Does it have to be in that order? > > > > I was thinking that the as long as there is a single +1 at any time in > that > > week (or after that week) then it would be good to merge > > > > On Tue, Jul 10, 2018 at 12:36 PM Robert Dale wrote: > > > > > There might be a better alternative to privately nagging ;-) Github > has > > a > > > feature on the sidebar that can be used to request reviews from > > individuals > > > or groups. The heading has 'Reviewers' and, when it's active, has a > gear > > > icon to select people. Github will then email the reviewers with the > > > request. It looks like its disabled for this project. > > > > > > I like the idea of adding the option of having a single vote and a week > > to > > > soak. Does it have to be in that order? Or can the vote take place > any > > > time during that week? Otherwise, you could end up with no one voting > in > > > the first week, get a vote, then waiting another week. > > > > > > Robert Dale > > > > > > > > > On Tue, Jul 10, 2018 at 7:22 AM Jorge Bay Gondra < > > jorgebaygon...@gmail.com > > > > > > > wrote: > > > > > > > I'm +1 on the idea of switching to lazy consensus after a single > > binding > > > > plus one and a week for objection. TinkerPop has so many different > > > modules > > > > / areas and committers have different expertise that is hard to get 3 > > > votes > > > > on something. > > > > > > > > Other projects have the concept of main "reviewer" and this would be > > very > > > > similar, a committer that was responsible for reviewing the code and > to > > > > check that everything is in order. > > > > > > > > El mar., 10 jul. 2018 a las 13:01, Stephen Mallette (< > > > spmalle...@gmail.com > > > > >) > > > > escribió: > > > > > > > > > I believe that the review process is not working so well anymore. > I'm > > > not > > > > > sure if committers/PMC members have just not had time to do reviews > > or > > > > have > > > > > not felt comfortable doing them, but for the most part they aren't > > > > getting > > > > > done and PRs are languishing. Personally, I like our process, but > if > > it > > > > > takes 3+ weeks to deal with a PR like this: > > > > > > > > > > https://github.com/apache/tinkerpop/pull/879/files > > > > > > > > > > where all we did was remove deprecated methods, I don't think we're > > > ever > > > > > going to get anything else through. As it stands, I personally > chase > > > > votes > > > > > in the background to get PRs to merge.and, I don't want to do > > that > > > > > anymore. > > > > > > > > > > I'll remind committers (and those interested in becoming > committers) > > > > that a > > > > > "review" in our context doesn't have to be a full on review of code > > > where > > > > > you hold this deep understanding of everything that is going on. > That > > > is > > > > > awesome when that happens, but it is perfectly fine to review/VOTE >
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
Github user mattallenuk commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/889#discussion_r201431625 --- Diff: gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js --- @@ -151,7 +156,16 @@ class DriverRemoteConnection extends RemoteConnection { return; } -if (response.status.code >= 400) { +if (response.status.code === responseStatusCode.authenticationChallenge && this._authenticator) { + this._authenticator.evaluateChallenge(response).then(res => { --- End diff -- On this problem, the issue is that submit creates a new requestId for each message sent. So when sending the auth response a new requestId is created for that authentication response. The Gremlin server responds with the requestId of the previous call so the authentication requestId never gets dealt with. Am I right in thinking that your proposal is to remove all pending handlers from the handler array? Is it safe to assume that there will only ever be one handler at a time in that array, apart from when authentication is happening? Am I making sense? ---
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
Github user mattallenuk commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/889#discussion_r201429698 --- Diff: gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/remote-connection.js --- @@ -33,9 +33,11 @@ class RemoteConnection { /** * @abstract * @param {Bytecode} bytecode + * @param {String} op Operation to perform, defaults to bytecode. + * @param {Object} args The arguments for the operation. Defaults to --- End diff -- Yep missed that, sorry :) ---
Re: [DISCUSS] Review Process
I fell way off the PR review train, I'll get back on. For clarification, is that a +1 on top of the submitter +1? I'm thinking you all just meant the submitter's +1 would be adequate after the lazy consensus period but wanted to be sure. I'd be fine to moving with that. My impression is that with the folks involved, if a submitter feels that another set of eyes is really required and lazy consensus is not adequate, regardless of the policy, that review will be sought and performed prior to merge. --Ted On Tue, Jul 10, 2018 at 11:44 AM Stephen Mallette wrote: > > It looks like its disabled for this project. > > I don't think we can use the GitHub integration without getting off our > Apache Mirror (which we've discussed, but not really pulled the trigger on > for no particular reason other than the hassle of changing everything). > > > Does it have to be in that order? > > I was thinking that the as long as there is a single +1 at any time in that > week (or after that week) then it would be good to merge > > On Tue, Jul 10, 2018 at 12:36 PM Robert Dale wrote: > > > There might be a better alternative to privately nagging ;-) Github has > a > > feature on the sidebar that can be used to request reviews from > individuals > > or groups. The heading has 'Reviewers' and, when it's active, has a gear > > icon to select people. Github will then email the reviewers with the > > request. It looks like its disabled for this project. > > > > I like the idea of adding the option of having a single vote and a week > to > > soak. Does it have to be in that order? Or can the vote take place any > > time during that week? Otherwise, you could end up with no one voting in > > the first week, get a vote, then waiting another week. > > > > Robert Dale > > > > > > On Tue, Jul 10, 2018 at 7:22 AM Jorge Bay Gondra < > jorgebaygon...@gmail.com > > > > > wrote: > > > > > I'm +1 on the idea of switching to lazy consensus after a single > binding > > > plus one and a week for objection. TinkerPop has so many different > > modules > > > / areas and committers have different expertise that is hard to get 3 > > votes > > > on something. > > > > > > Other projects have the concept of main "reviewer" and this would be > very > > > similar, a committer that was responsible for reviewing the code and to > > > check that everything is in order. > > > > > > El mar., 10 jul. 2018 a las 13:01, Stephen Mallette (< > > spmalle...@gmail.com > > > >) > > > escribió: > > > > > > > I believe that the review process is not working so well anymore. I'm > > not > > > > sure if committers/PMC members have just not had time to do reviews > or > > > have > > > > not felt comfortable doing them, but for the most part they aren't > > > getting > > > > done and PRs are languishing. Personally, I like our process, but if > it > > > > takes 3+ weeks to deal with a PR like this: > > > > > > > > https://github.com/apache/tinkerpop/pull/879/files > > > > > > > > where all we did was remove deprecated methods, I don't think we're > > ever > > > > going to get anything else through. As it stands, I personally chase > > > votes > > > > in the background to get PRs to merge.and, I don't want to do > that > > > > anymore. > > > > > > > > I'll remind committers (and those interested in becoming committers) > > > that a > > > > "review" in our context doesn't have to be a full on review of code > > where > > > > you hold this deep understanding of everything that is going on. That > > is > > > > awesome when that happens, but it is perfectly fine to review/VOTE in > > the > > > > following manner (as examples): > > > > > > > > + VOTE +1 - ran docker integration tests and everything passes > > > > + VOTE +1 - reviewed the code in detail - solid pull request > > > > + VOTE +1 - agree with the principle of this pull request but don't > > fully > > > > understand the code > > > > + VOTE +1 - read through the updated documentation and understand why > > > this > > > > is important, nice > > > > > > > > So basically, you can VOTE and just explain your position for why you > > > voted > > > > (or not explain). I would like to keep this process, however, if we > > can't > > > > raise the VOTEs for whatever reason, then I'd like to suggest a > change. > > > > > > > > I'd suggest that we go to a slightly looser version of > > > review-then-commit, > > > > where we require the 3 binding +1 VOTEs as we have been doing OR we > > > > require a single binding +1 and 1 week for objection at which point > we > > > have > > > > a form of lazy consensus. > > > > > > > > Honestly, I'd like to see some discussion on this from committers/PMC > > > > members and not go with the standard form of lazy consensus that we > > > > typically end up with. However, if no one truly has anything to say, > > > > consider the 72 hours started now. > > > > > > > > > >
[jira] [Updated] (TINKERPOP-2002) Create a blog post explaining the value of using TinkerPop
[ https://issues.apache.org/jira/browse/TINKERPOP-2002?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] stephen mallette updated TINKERPOP-2002: Affects Version/s: 3.2.9 Issue Type: Improvement (was: Wish) > Create a blog post explaining the value of using TinkerPop > -- > > Key: TINKERPOP-2002 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2002 > Project: TinkerPop > Issue Type: Improvement > Components: documentation >Affects Versions: 3.2.9 >Reporter: Jeremy Hanna >Priority: Major > > There are a lot of great references and recipes in the TinkerPop docs. One > thing that would be nice to have is a simple explanation of what it means to > use a TinkerPop implementation and what that guarantees. People coming from a > relational world may misunderstand what to expect and might think they can do > the equivalent of plugging in a new JDBC driver and away they go. > Some things to include > * How traversals are implementation agnostic with some caveats > ** data types > ** indexes > * Connection options - different implementations may allow you to use > TinkerPop drivers but may have implementation specific drivers for things > like security or cluster awareness > * How to migrate easily between TinkerPop implementations should the need > arise -- This message was sent by Atlassian JIRA (v7.6.3#76005)
Re: [DISCUSS] Review Process
> It looks like its disabled for this project. I don't think we can use the GitHub integration without getting off our Apache Mirror (which we've discussed, but not really pulled the trigger on for no particular reason other than the hassle of changing everything). > Does it have to be in that order? I was thinking that the as long as there is a single +1 at any time in that week (or after that week) then it would be good to merge On Tue, Jul 10, 2018 at 12:36 PM Robert Dale wrote: > There might be a better alternative to privately nagging ;-) Github has a > feature on the sidebar that can be used to request reviews from individuals > or groups. The heading has 'Reviewers' and, when it's active, has a gear > icon to select people. Github will then email the reviewers with the > request. It looks like its disabled for this project. > > I like the idea of adding the option of having a single vote and a week to > soak. Does it have to be in that order? Or can the vote take place any > time during that week? Otherwise, you could end up with no one voting in > the first week, get a vote, then waiting another week. > > Robert Dale > > > On Tue, Jul 10, 2018 at 7:22 AM Jorge Bay Gondra > > wrote: > > > I'm +1 on the idea of switching to lazy consensus after a single binding > > plus one and a week for objection. TinkerPop has so many different > modules > > / areas and committers have different expertise that is hard to get 3 > votes > > on something. > > > > Other projects have the concept of main "reviewer" and this would be very > > similar, a committer that was responsible for reviewing the code and to > > check that everything is in order. > > > > El mar., 10 jul. 2018 a las 13:01, Stephen Mallette (< > spmalle...@gmail.com > > >) > > escribió: > > > > > I believe that the review process is not working so well anymore. I'm > not > > > sure if committers/PMC members have just not had time to do reviews or > > have > > > not felt comfortable doing them, but for the most part they aren't > > getting > > > done and PRs are languishing. Personally, I like our process, but if it > > > takes 3+ weeks to deal with a PR like this: > > > > > > https://github.com/apache/tinkerpop/pull/879/files > > > > > > where all we did was remove deprecated methods, I don't think we're > ever > > > going to get anything else through. As it stands, I personally chase > > votes > > > in the background to get PRs to merge.and, I don't want to do that > > > anymore. > > > > > > I'll remind committers (and those interested in becoming committers) > > that a > > > "review" in our context doesn't have to be a full on review of code > where > > > you hold this deep understanding of everything that is going on. That > is > > > awesome when that happens, but it is perfectly fine to review/VOTE in > the > > > following manner (as examples): > > > > > > + VOTE +1 - ran docker integration tests and everything passes > > > + VOTE +1 - reviewed the code in detail - solid pull request > > > + VOTE +1 - agree with the principle of this pull request but don't > fully > > > understand the code > > > + VOTE +1 - read through the updated documentation and understand why > > this > > > is important, nice > > > > > > So basically, you can VOTE and just explain your position for why you > > voted > > > (or not explain). I would like to keep this process, however, if we > can't > > > raise the VOTEs for whatever reason, then I'd like to suggest a change. > > > > > > I'd suggest that we go to a slightly looser version of > > review-then-commit, > > > where we require the 3 binding +1 VOTEs as we have been doing OR we > > > require a single binding +1 and 1 week for objection at which point we > > have > > > a form of lazy consensus. > > > > > > Honestly, I'd like to see some discussion on this from committers/PMC > > > members and not go with the standard form of lazy consensus that we > > > typically end up with. However, if no one truly has anything to say, > > > consider the 72 hours started now. > > > > > >
Re: [DISCUSS] Review Process
There might be a better alternative to privately nagging ;-) Github has a feature on the sidebar that can be used to request reviews from individuals or groups. The heading has 'Reviewers' and, when it's active, has a gear icon to select people. Github will then email the reviewers with the request. It looks like its disabled for this project. I like the idea of adding the option of having a single vote and a week to soak. Does it have to be in that order? Or can the vote take place any time during that week? Otherwise, you could end up with no one voting in the first week, get a vote, then waiting another week. Robert Dale On Tue, Jul 10, 2018 at 7:22 AM Jorge Bay Gondra wrote: > I'm +1 on the idea of switching to lazy consensus after a single binding > plus one and a week for objection. TinkerPop has so many different modules > / areas and committers have different expertise that is hard to get 3 votes > on something. > > Other projects have the concept of main "reviewer" and this would be very > similar, a committer that was responsible for reviewing the code and to > check that everything is in order. > > El mar., 10 jul. 2018 a las 13:01, Stephen Mallette ( >) > escribió: > > > I believe that the review process is not working so well anymore. I'm not > > sure if committers/PMC members have just not had time to do reviews or > have > > not felt comfortable doing them, but for the most part they aren't > getting > > done and PRs are languishing. Personally, I like our process, but if it > > takes 3+ weeks to deal with a PR like this: > > > > https://github.com/apache/tinkerpop/pull/879/files > > > > where all we did was remove deprecated methods, I don't think we're ever > > going to get anything else through. As it stands, I personally chase > votes > > in the background to get PRs to merge.and, I don't want to do that > > anymore. > > > > I'll remind committers (and those interested in becoming committers) > that a > > "review" in our context doesn't have to be a full on review of code where > > you hold this deep understanding of everything that is going on. That is > > awesome when that happens, but it is perfectly fine to review/VOTE in the > > following manner (as examples): > > > > + VOTE +1 - ran docker integration tests and everything passes > > + VOTE +1 - reviewed the code in detail - solid pull request > > + VOTE +1 - agree with the principle of this pull request but don't fully > > understand the code > > + VOTE +1 - read through the updated documentation and understand why > this > > is important, nice > > > > So basically, you can VOTE and just explain your position for why you > voted > > (or not explain). I would like to keep this process, however, if we can't > > raise the VOTEs for whatever reason, then I'd like to suggest a change. > > > > I'd suggest that we go to a slightly looser version of > review-then-commit, > > where we require the 3 binding +1 VOTEs as we have been doing OR we > > require a single binding +1 and 1 week for objection at which point we > have > > a form of lazy consensus. > > > > Honestly, I'd like to see some discussion on this from committers/PMC > > members and not go with the standard form of lazy consensus that we > > typically end up with. However, if no one truly has anything to say, > > consider the 72 hours started now. > > >
[jira] [Created] (TINKERPOP-2002) Create a blog post explaining the value of using TinkerPop
Jeremy Hanna created TINKERPOP-2002: --- Summary: Create a blog post explaining the value of using TinkerPop Key: TINKERPOP-2002 URL: https://issues.apache.org/jira/browse/TINKERPOP-2002 Project: TinkerPop Issue Type: Wish Components: documentation Reporter: Jeremy Hanna There are a lot of great references and recipes in the TinkerPop docs. One thing that would be nice to have is a simple explanation of what it means to use a TinkerPop implementation and what that guarantees. People coming from a relational world may misunderstand what to expect and might think they can do the equivalent of plugging in a new JDBC driver and away they go. Some things to include * How traversals are implementation agnostic with some caveats ** data types ** indexes * Connection options - different implementations may allow you to use TinkerPop drivers but may have implementation specific drivers for things like security or cluster awareness * How to migrate easily between TinkerPop implementations should the need arise -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TINKERPOP-1913) Expose metadata from Gremlin Server to Clients
[ https://issues.apache.org/jira/browse/TINKERPOP-1913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16538653#comment-16538653 ] stephen mallette commented on TINKERPOP-1913: - Linked the TINKERPOP-1906 ticket - there are still a few things left on that body of work before it can merge. > Expose metadata from Gremlin Server to Clients > -- > > Key: TINKERPOP-1913 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1913 > Project: TinkerPop > Issue Type: Improvement > Components: dotnet, driver, javascript, python, server >Affects Versions: 3.3.1 >Reporter: Ashwini Singh >Assignee: stephen mallette >Priority: Major > > To summarize what we have discussed so far: > 1. For API using GremlinRequest/QueryScript, expose response attribute as > part of result. Using an approach to similar to Java client driver (using > ResultSet) . [Priority0] > -- We rely on the last message for response attributes. > 2. For GLV, add response attribute as part of Traversal. [Priority 0] > --Rely on the last message for attributes. > 3. Expose other server details (like server setting). I would suggest to > split this design discussion into two directions: > a. Metadata for request execution: Only focuses on details > related to request execution. Can be achieved through #1 and #2. > b. Metadata for Gremlin Server: Focuses on overall metadata > for the server. Could be a separate request and fetch the data once for a > connection. > Targeted drivers: dotnet, Java, python, javascript. > More details: > [https://lists.apache.org/thread.html/fd2208a2db827bc1eb479ad8c2f181bd2fa532553c97b3fe6994a7b6@%3Cdev.tinkerpop.apache.org%3E] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TINKERPOP-1913) Expose metadata from Gremlin Server to Clients
[ https://issues.apache.org/jira/browse/TINKERPOP-1913?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16538518#comment-16538518 ] Roman Kreisel commented on TINKERPOP-1913: -- Please note, that solving this ticket would also close TINKERPOP-1906 for me. It would be really great, if [~Ashwini Singh]'s patch would become part of 3.3.4 Best Regards Roman > Expose metadata from Gremlin Server to Clients > -- > > Key: TINKERPOP-1913 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1913 > Project: TinkerPop > Issue Type: Improvement > Components: dotnet, driver, javascript, python, server >Affects Versions: 3.3.1 >Reporter: Ashwini Singh >Assignee: stephen mallette >Priority: Major > > To summarize what we have discussed so far: > 1. For API using GremlinRequest/QueryScript, expose response attribute as > part of result. Using an approach to similar to Java client driver (using > ResultSet) . [Priority0] > -- We rely on the last message for response attributes. > 2. For GLV, add response attribute as part of Traversal. [Priority 0] > --Rely on the last message for attributes. > 3. Expose other server details (like server setting). I would suggest to > split this design discussion into two directions: > a. Metadata for request execution: Only focuses on details > related to request execution. Can be achieved through #1 and #2. > b. Metadata for Gremlin Server: Focuses on overall metadata > for the server. Could be a separate request and fetch the data once for a > connection. > Targeted drivers: dotnet, Java, python, javascript. > More details: > [https://lists.apache.org/thread.html/fd2208a2db827bc1eb479ad8c2f181bd2fa532553c97b3fe6994a7b6@%3Cdev.tinkerpop.apache.org%3E] > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Commented] (TINKERPOP-1986) Remove deprecation from PartitionStrategy, SubgraphStrategy and GremlinScriptEngine
[ https://issues.apache.org/jira/browse/TINKERPOP-1986?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16538445#comment-16538445 ] ASF GitHub Bot commented on TINKERPOP-1986: --- Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/879 BTW, regardless of the ongoing discussion, my VOTE: +1 :) > Remove deprecation from PartitionStrategy, SubgraphStrategy and > GremlinScriptEngine > --- > > Key: TINKERPOP-1986 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1986 > Project: TinkerPop > Issue Type: Improvement > Components: process >Affects Versions: 3.3.3 >Reporter: stephen mallette >Assignee: stephen mallette >Priority: Minor > Labels: breaking > Fix For: 3.4.0 > > > {{PartitionStrategy}}, {{SubgraphStrategy}} and {{GremlinScriptEngine}} all > have some deprecated code that should be removed. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] tinkerpop issue #879: TINKERPOP-1986 Removed deprecated methods
Github user jorgebay commented on the issue: https://github.com/apache/tinkerpop/pull/879 BTW, regardless of the ongoing discussion, my VOTE: +1 :) ---
[jira] [Commented] (TINKERPOP-1995) DriverRemoteConnection close() method returns undefined
[ https://issues.apache.org/jira/browse/TINKERPOP-1995?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16538433#comment-16538433 ] ASF GitHub Bot commented on TINKERPOP-1995: --- Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/888 VOTE +1 > DriverRemoteConnection close() method returns undefined > --- > > Key: TINKERPOP-1995 > URL: https://issues.apache.org/jira/browse/TINKERPOP-1995 > Project: TinkerPop > Issue Type: Improvement > Components: javascript >Affects Versions: 3.3.3, 3.2.9 >Reporter: Jorge Bay >Priority: Major > Fix For: 3.4.0, 3.3.4, 3.2.10 > > > {{close()}} instance method should return a {{Promise}}, instead it returns > {{undefined}}. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] tinkerpop issue #888: TINKERPOP-1995 Return the Promise on Close
Github user spmallette commented on the issue: https://github.com/apache/tinkerpop/pull/888 VOTE +1 ---
Re: [DISCUSS] Review Process
I'm +1 on the idea of switching to lazy consensus after a single binding plus one and a week for objection. TinkerPop has so many different modules / areas and committers have different expertise that is hard to get 3 votes on something. Other projects have the concept of main "reviewer" and this would be very similar, a committer that was responsible for reviewing the code and to check that everything is in order. El mar., 10 jul. 2018 a las 13:01, Stephen Mallette () escribió: > I believe that the review process is not working so well anymore. I'm not > sure if committers/PMC members have just not had time to do reviews or have > not felt comfortable doing them, but for the most part they aren't getting > done and PRs are languishing. Personally, I like our process, but if it > takes 3+ weeks to deal with a PR like this: > > https://github.com/apache/tinkerpop/pull/879/files > > where all we did was remove deprecated methods, I don't think we're ever > going to get anything else through. As it stands, I personally chase votes > in the background to get PRs to merge.and, I don't want to do that > anymore. > > I'll remind committers (and those interested in becoming committers) that a > "review" in our context doesn't have to be a full on review of code where > you hold this deep understanding of everything that is going on. That is > awesome when that happens, but it is perfectly fine to review/VOTE in the > following manner (as examples): > > + VOTE +1 - ran docker integration tests and everything passes > + VOTE +1 - reviewed the code in detail - solid pull request > + VOTE +1 - agree with the principle of this pull request but don't fully > understand the code > + VOTE +1 - read through the updated documentation and understand why this > is important, nice > > So basically, you can VOTE and just explain your position for why you voted > (or not explain). I would like to keep this process, however, if we can't > raise the VOTEs for whatever reason, then I'd like to suggest a change. > > I'd suggest that we go to a slightly looser version of review-then-commit, > where we require the 3 binding +1 VOTEs as we have been doing OR we > require a single binding +1 and 1 week for objection at which point we have > a form of lazy consensus. > > Honestly, I'd like to see some discussion on this from committers/PMC > members and not go with the standard form of lazy consensus that we > typically end up with. However, if no one truly has anything to say, > consider the 72 hours started now. >
Re: ASF Board Draft Report - July 2018
I've submitted the report for July 2018 to the board. On Mon, Jul 2, 2018 at 8:42 AM Stephen Mallette wrote: > Here is the draft of the January 2018 board report - please let me know if > there is anything else to add. > > > --- > > ## Description: > Apache TinkerPop is a graph computing framework for both graph databases > (OLTP) and graph analytic systems (OLAP). > > ## Activity: > TinkerPop released versions 3.2.9 and 3.3.3 in early May which included > many bug fixes and some minor new features. Development has started on the > next major release, 3.4.0, which will contain major new features and some > breaking API changes, the first such release in about a year (3.3.0 in > August of 2017 was the last time we had this type of release). Once 3.4.0 > is released in coming months, the community will discuss what the future > plan is for the 3.2.x line and whether it requires continued support. We > will continue to develop the 3.3.x line for some time. When 3.4.0 releases > we will have companion 3.2.10 and 3.3.4 releases as well. > > TinkerPop has seen continued growth in the development of third-party > libraries, which we tend to take as a sign of good community health. Two > recently announced examples include: > > * spring-data-gremlin[1] > * kotlin-gremlin-ogm[2] > > Both of these libraries open TinkerPop and its graph query language, > Gremlin, to entirely new development ecosystems in Spring Data and Kotlin > respectively. > > ## Issues: > There are no issues requiring board attention at this time. > > ## Releases: > - 3.2.9 (May 8, 2018) > - 3.3.3 (May 8, 2018) > > ## PMC/Committer: > - Last PMC addition was Robert Dale - April 2017 > - Last committer addition was Kelvin Lawrence - December 2017 > > ## Links > > [1] https://github.com/Microsoft/spring-data-gremlin > [2] https://github.com/pm-dev/kotlin-gremlin-ogm >
[DISCUSS] Review Process
I believe that the review process is not working so well anymore. I'm not sure if committers/PMC members have just not had time to do reviews or have not felt comfortable doing them, but for the most part they aren't getting done and PRs are languishing. Personally, I like our process, but if it takes 3+ weeks to deal with a PR like this: https://github.com/apache/tinkerpop/pull/879/files where all we did was remove deprecated methods, I don't think we're ever going to get anything else through. As it stands, I personally chase votes in the background to get PRs to merge.and, I don't want to do that anymore. I'll remind committers (and those interested in becoming committers) that a "review" in our context doesn't have to be a full on review of code where you hold this deep understanding of everything that is going on. That is awesome when that happens, but it is perfectly fine to review/VOTE in the following manner (as examples): + VOTE +1 - ran docker integration tests and everything passes + VOTE +1 - reviewed the code in detail - solid pull request + VOTE +1 - agree with the principle of this pull request but don't fully understand the code + VOTE +1 - read through the updated documentation and understand why this is important, nice So basically, you can VOTE and just explain your position for why you voted (or not explain). I would like to keep this process, however, if we can't raise the VOTEs for whatever reason, then I'd like to suggest a change. I'd suggest that we go to a slightly looser version of review-then-commit, where we require the 3 binding +1 VOTEs as we have been doing OR we require a single binding +1 and 1 week for objection at which point we have a form of lazy consensus. Honestly, I'd like to see some discussion on this from committers/PMC members and not go with the standard form of lazy consensus that we typically end up with. However, if no one truly has anything to say, consider the 72 hours started now.
[jira] [Created] (TINKERPOP-2001) JavaScript GLV: Support Groovy and Python lambdas
Jorge Bay created TINKERPOP-2001: Summary: JavaScript GLV: Support Groovy and Python lambdas Key: TINKERPOP-2001 URL: https://issues.apache.org/jira/browse/TINKERPOP-2001 Project: TinkerPop Issue Type: Improvement Components: javascript Reporter: Jorge Bay Similar to TINKERPOP-1854, we should add support for string representations of groovy/python lambdas. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
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 { /** * @abstract * @param {Bytecode} bytecode + * @param {String} op Operation to perform, defaults to bytecode. + * @param {Object} args The arguments for the operation. Defaults to --- End diff -- Missing jsdoc :) "Defaults to an associative array containing values for "aliases" and "gremlin" keys. ---
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
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 DriverRemoteConnection extends RemoteConnection { } /** @override */ - submit(bytecode) { + submit(bytecode, op, args) { return this.open().then(() => new Promise((resolve, reject) => { - const requestId = getUuid(); + const requestId = utils.getUuid(); this._responseHandlers[requestId] = { callback: (err, result) => err ? reject(err) : resolve(result), result: null }; - const message = bufferFromString(this._header + JSON.stringify(this._getRequest(requestId, bytecode))); + const message = bufferFromString(this._header + JSON.stringify(this._getRequest(requestId, bytecode, op, args))); this._ws.send(message); })); } - _getRequest(id, bytecode) { + _getRequest(id, bytecode, op, args) { return ({ 'requestId': { '@type': 'g:UUID', '@value': id }, - 'op': 'bytecode', + 'op': op || 'bytecode', 'processor': 'traversal', - 'args': { + 'args': args || { --- End diff -- I think each value of args map should be encoded with `this._writer.adaptObject()` ---
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
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 DriverRemoteConnection extends RemoteConnection { return; } -if (response.status.code >= 400) { +if (response.status.code === responseStatusCode.authenticationChallenge && this._authenticator) { + this._authenticator.evaluateChallenge(response).then(res => { +this.submit('', 'authentication', res); --- End diff -- Just a nit: we can use `null` instead of empty string to denote no `bytecode`. ---
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
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 DriverRemoteConnection extends RemoteConnection { return; } -if (response.status.code >= 400) { +if (response.status.code === responseStatusCode.authenticationChallenge && this._authenticator) { + this._authenticator.evaluateChallenge(response).then(res => { --- End diff -- To avoid leaking those response handlers, you should call `_clearHandler()`: ```javascript // Similar to how its being done below, maybe we can move that to a separate method this._clearHandler(response.requestId); handler.callback(null, { traversers: [] }); ``` So the final solution should be something like: ```javascript this._authenticator.evaluateChallenge(response) .then(() => this.submit(null, 'authentication', res)) .then(() => this._clearHandlerAndInvokeCallback(response.requestId, handler, [])) .catch(handler.callback); return; ``` ---
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
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 DriverRemoteConnection extends RemoteConnection { return; } -if (response.status.code >= 400) { +if (response.status.code === responseStatusCode.authenticationChallenge && this._authenticator) { + this._authenticator.evaluateChallenge(response).then(res => { +this.submit('', 'authentication', res); + }, err => { +return handler.callback(err); + }); + + return; --- End diff -- NIT: it looks like there is an extra spacing here :) ---
[GitHub] tinkerpop pull request #889: Tinkerpop 1977 - Sasl Authentication
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 DriverRemoteConnection extends RemoteConnection { return; } -if (response.status.code >= 400) { +if (response.status.code === responseStatusCode.authenticationChallenge && this._authenticator) { + this._authenticator.evaluateChallenge(response).then(res => { --- End diff -- We can capture any possible error if we use a chained `catch()`, instead of `then()` with multiple callback functions: ``` this._authenticator.evaluateChallenge(response) .then(() => { // Any possible error thrown or promise rejection will be captured return this.submit('', 'authentication', res); }) .catch(handler.callback) ---
[jira] [Commented] (TINKERPOP-2000) MANIFEST.MF contains invalid key
[ https://issues.apache.org/jira/browse/TINKERPOP-2000?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16538376#comment-16538376 ] stephen mallette commented on TINKERPOP-2000: - It's a custom key, stamped into the manifest file and simply refers to the "TinkerPop Version" - if there is a standard key that specifies that then I'm fine to change it. I seem to recall there being some discussion about Specification-Version (and other "-Version" standard fields) not being the right thing to use for this case which is why we have this custom one. If you have some knowledge to share in this area please let us know. > MANIFEST.MF contains invalid key > > > Key: TINKERPOP-2000 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2000 > Project: TinkerPop > Issue Type: Improvement > Components: build-release >Affects Versions: 3.3.3 >Reporter: Horacio Hoyos Rodriguez >Priority: Critical > > The MANIFEST.MF file for tinerpop-core contains an invalid key (line 8): > version: 3.3.3 > Some tools (like the Eclipse Bundle Recipes) check for the MANIFEST integrity > and this line causes an error. Perhaps this is the Specification-Version? > -- This message was sent by Atlassian JIRA (v7.6.3#76005)
[jira] [Created] (TINKERPOP-2000) MANIFEST.MF contains invalid key
Horacio Hoyos Rodriguez created TINKERPOP-2000: -- Summary: MANIFEST.MF contains invalid key Key: TINKERPOP-2000 URL: https://issues.apache.org/jira/browse/TINKERPOP-2000 Project: TinkerPop Issue Type: Improvement Components: build-release Affects Versions: 3.3.3 Reporter: Horacio Hoyos Rodriguez The MANIFEST.MF file for tinerpop-core contains an invalid key (line 8): version: 3.3.3 Some tools (like the Eclipse Bundle Recipes) check for the MANIFEST integrity and this line causes an error. Perhaps this is the Specification-Version? -- This message was sent by Atlassian JIRA (v7.6.3#76005)