You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@jena.apache.org by ajs6f <gi...@git.apache.org> on 2015/12/08 17:54:22 UTC
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
GitHub user ajs6f opened a pull request:
https://github.com/apache/jena/pull/105
JENA-1078: Logging warning when transaction is ended without commiting
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ajs6f/jena AddNoCommitWarning
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/jena/pull/105.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #105
----
commit 53975ed362e27c88cea7326789fad5c1d009e98e
Author: ajs6f <aj...@virginia.edu>
Date: 2015-12-08T16:53:25Z
Logging warning when transaction is ended without commiting
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on a diff in the pull request:
https://github.com/apache/jena/pull/105#discussion_r47078789
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/mem/DatasetGraphInMemory.java ---
@@ -161,17 +161,17 @@ public void abort() {
@Override
public void close() {
if (isInTransaction()) abort();
-
}
@Override
public void end() {
if (isInTransaction()) {
+ log.warn("Ending transaction without commit!");
--- End diff --
begin(READ)-end() is a normal pattern. The code needs to know if it is considering a READ or WRITE transaction.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/105#issuecomment-163944788
We have had the discussion about warnings again on the dev@ just recently.
I've already started on a review and some fixes. There are some other problems with abort/end. I will submit a PR here for detailed code review. The issues look harmless but that's because of the way the implementation of indexes happen to work. Different plugged in indexes would not be so safe.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/jena/pull/105
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on a diff in the pull request:
https://github.com/apache/jena/pull/105#discussion_r47086225
--- Diff: jena-arq/src/main/java/org/apache/jena/sparql/core/mem/DatasetGraphInMemory.java ---
@@ -161,17 +161,17 @@ public void abort() {
@Override
public void close() {
if (isInTransaction()) abort();
-
}
@Override
public void end() {
if (isInTransaction()) {
+ log.warn("Ending transaction without commit!");
--- End diff --
Fixed with latest commit.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the pull request:
https://github.com/apache/jena/pull/105#issuecomment-163937760
I had no idea that throwing WARNINGs in the test suite was any kind of issue. ERRORs, yes, but not WARNINGs. That's a policy that might be recorded somewhere, perhaps [here](https://jena.apache.org/getting_involved/reviewing_contributions.html)?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
Posted by ajs6f <gi...@git.apache.org>.
Github user ajs6f commented on the pull request:
https://github.com/apache/jena/pull/105#issuecomment-163938075
In any event, I can review them and eliminate them. No need for you to spend time doing that-- this is my code and I take responsibility for it. Would you reopen the ticket for me?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] jena pull request: JENA-1078: Logging warning when transaction is ...
Posted by afs <gi...@git.apache.org>.
Github user afs commented on the pull request:
https://github.com/apache/jena/pull/105#issuecomment-163936807
This causes about 12 warnings from the test suite (Eclipse and maven).
Please do add tests and check the test suite runs. I will check them and fix or silence each one but it would be nice to be at least notified that it would happen.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---