You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by GitBox <gi...@apache.org> on 2022/06/16 18:57:58 UTC
[GitHub] [tinkerpop] L0Lmaker opened a new pull request, #1714: TINKERPOP-2751: Throw exception if transaction attempted on non transaction supported Graph
L0Lmaker opened a new pull request, #1714:
URL: https://github.com/apache/tinkerpop/pull/1714
Link to issue: [TINKERPOP 2751](https://issues.apache.org/jira/browse/TINKERPOP-2751)
If a transaction is attempted on a graph that does not support transactions, the client hangs as it waits for a response from the server. The change in this PR handles that case and responds to the client with a `transactionNotSupported()` exception.
Ideally, we would want the client to fail at `transaction.begin()` but this would involve a much bigger change on the server and across the GLVs. For now, users should be notified that they need to use a transaction-enabled graph when attempting to perform a graph operation (rollback or commit)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on a diff in pull request #1714: TINKERPOP-2751: Throw exception if transaction attempted on non transaction supported Graph
Posted by GitBox <gi...@apache.org>.
spmallette commented on code in PR #1714:
URL: https://github.com/apache/tinkerpop/pull/1714#discussion_r900095906
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java:
##########
@@ -526,6 +526,11 @@ protected void handleGraphOperation(final Bytecode bytecode, final Graph graph,
throw new IllegalStateException(String.format(
"Bytecode in request is not a recognized graph operation: %s", bytecode.toString()));
}
+ } else {
+ UnsupportedOperationException uoe = Graph.Exceptions.transactionsNotSupported();
Review Comment:
nit: final
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] L0Lmaker commented on a diff in pull request #1714: TINKERPOP-2751: Throw exception if transaction attempted on non transaction supported Graph
Posted by GitBox <gi...@apache.org>.
L0Lmaker commented on code in PR #1714:
URL: https://github.com/apache/tinkerpop/pull/1714#discussion_r901888980
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/SessionOpProcessor.java:
##########
@@ -526,6 +526,11 @@ protected void handleGraphOperation(final Bytecode bytecode, final Graph graph,
throw new IllegalStateException(String.format(
"Bytecode in request is not a recognized graph operation: %s", bytecode.toString()));
}
+ } else {
+ UnsupportedOperationException uoe = Graph.Exceptions.transactionsNotSupported();
Review Comment:
converted to just a throw in [5b83100](https://github.com/apache/tinkerpop/pull/1714/commits/5b831000b54dcb535aa90616217726c3cc4a6dec)
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] codecov-commenter commented on pull request #1714: TINKERPOP-2751: Throw exception if transaction attempted on non transaction supported Graph
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #1714:
URL: https://github.com/apache/tinkerpop/pull/1714#issuecomment-1158048062
# [Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1714?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#1714](https://codecov.io/gh/apache/tinkerpop/pull/1714?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (1515212) into [3.5-dev](https://codecov.io/gh/apache/tinkerpop/commit/60dd5f966a14965bedc56e0dd9abaa2b295061b4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (60dd5f9) will **increase** coverage by `0.26%`.
> The diff coverage is `72.00%`.
```diff
@@ Coverage Diff @@
## 3.5-dev #1714 +/- ##
===========================================
+ Coverage 63.00% 63.27% +0.26%
===========================================
Files 23 23
Lines 3411 3553 +142
===========================================
+ Hits 2149 2248 +99
- Misses 1109 1131 +22
- Partials 153 174 +21
```
| [Impacted Files](https://codecov.io/gh/apache/tinkerpop/pull/1714?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [gremlin-go/driver/connection.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvY29ubmVjdGlvbi5nbw==) | `80.00% <0.00%> (-1.16%)` | :arrow_down: |
| [gremlin-go/driver/error\_codes.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvZXJyb3JfY29kZXMuZ28=) | `100.00% <ø> (ø)` | |
| [gremlin-go/driver/performance/performanceSuite.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvcGVyZm9ybWFuY2UvcGVyZm9ybWFuY2VTdWl0ZS5nbw==) | `0.00% <ø> (ø)` | |
| [gremlin-go/driver/protocol.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvcHJvdG9jb2wuZ28=) | `63.20% <0.00%> (+0.99%)` | :arrow_up: |
| [gremlin-go/driver/resultSet.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvcmVzdWx0U2V0Lmdv) | `80.18% <18.75%> (-10.14%)` | :arrow_down: |
| [gremlin-go/driver/graphBinary.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvZ3JhcGhCaW5hcnkuZ28=) | `53.79% <64.41%> (+2.75%)` | :arrow_up: |
| [gremlin-go/driver/strategies.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvc3RyYXRlZ2llcy5nbw==) | `65.69% <64.78%> (-1.48%)` | :arrow_down: |
| [gremlin-go/driver/traversal.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvdHJhdmVyc2FsLmdv) | `70.54% <83.33%> (+0.23%)` | :arrow_up: |
| [gremlin-go/driver/graphTraversalSource.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvZ3JhcGhUcmF2ZXJzYWxTb3VyY2UuZ28=) | `80.89% <85.00%> (ø)` | |
| [gremlin-go/driver/graphTraversal.go](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1nby9kcml2ZXIvZ3JhcGhUcmF2ZXJzYWwuZ28=) | `89.11% <89.62%> (+0.77%)` | :arrow_up: |
| ... and [8 more](https://codecov.io/gh/apache/tinkerpop/pull/1714/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1714?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1714?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [8691929...1515212](https://codecov.io/gh/apache/tinkerpop/pull/1714?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] spmallette commented on pull request #1714: TINKERPOP-2751: Throw exception if transaction attempted on non transaction supported Graph
Posted by GitBox <gi...@apache.org>.
spmallette commented on PR #1714:
URL: https://github.com/apache/tinkerpop/pull/1714#issuecomment-1158849792
The obvious downside here is that a non-transactional graph like TinkerGraph will have already recorded changes by this point so if your expectation was a "transaction" you didn't get that. On the other hand, this is how it would work in embedded mode as well. TinkerGraph wouldn't complain until you tried to `commit()`:
```text
gremlin> g = TinkerGraph.open().traversal()
==>graphtraversalsource[tinkergraph[vertices:0 edges:0], standard]
gremlin> g.addV()
==>v[0]
gremlin> g.addV()
==>v[1]
gremlin> g.tx().commit()
Graph does not support transactions
Type ':help' or ':h' for help.
Display stack trace? [yN]n
gremlin> g.V().count()
==>2
```
So in that sense remote behaves just like embedded so at least the behavior is consistent.
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.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] bechbd merged pull request #1714: TINKERPOP-2751: Throw exception if transaction attempted on non transaction supported Graph
Posted by GitBox <gi...@apache.org>.
bechbd merged PR #1714:
URL: https://github.com/apache/tinkerpop/pull/1714
--
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.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] lyndonbauto commented on pull request #1714: TINKERPOP-2751: Throw exception if transaction attempted on non transaction supported Graph
Posted by GitBox <gi...@apache.org>.
lyndonbauto commented on PR #1714:
URL: https://github.com/apache/tinkerpop/pull/1714#issuecomment-1160993595
Nice - I remember briefly looking for the culprit on this issue. 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.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org