You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nifi.apache.org by markap14 <gi...@git.apache.org> on 2016/05/18 17:19:20 UTC
[GitHub] nifi pull request: NIFI-1745: Refactor how revisions are handled a...
GitHub user markap14 opened a pull request:
https://github.com/apache/nifi/pull/454
NIFI-1745: Refactor how revisions are handled at NCM/Distributed to Node
This is part 2 of NIFI-1745, which provides user info to revision claims/locks and removes the cluster context, as it is no longer necessary
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/markap14/nifi NIFI-1745-Part2
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/454.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 #454
----
commit 55fb1dab52880cfba94ac6ebd3552b4019020b1b
Author: Mark Payne <ma...@hotmail.com>
Date: 2016-05-17T15:51:09Z
NIFI-1745: Refactor how revisions are handled at NCM/Distributed to Node
----
---
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] nifi pull request: NIFI-1745: Refactor how revisions are handled a...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the pull request:
https://github.com/apache/nifi/pull/454#issuecomment-220330762
@markap14 When canceling revision claims for Connectable components in the verifyXxx() methods, DAO access will not return null so you do not need to check for it there. A ResourceNotFoundException will be thrown. Is that acceptable given it's usage?
---
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] nifi pull request: NIFI-1745: Refactor how revisions are handled a...
Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on the pull request:
https://github.com/apache/nifi/pull/454#issuecomment-220610191
@mcgilman I agree, I do think it makes more sense to handle that in the Revision. Have updated the PR to back that out so that we can instead do it in the Resource in the next PR.
---
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] nifi pull request: NIFI-1745: Refactor how revisions are handled a...
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/454#discussion_r63874050
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java ---
@@ -423,7 +440,7 @@ public boolean requestWriteLock(final Revision proposedRevision) throws ExpiredR
throw ise;
}
- if (stamp.getClientId() == null || stamp.getClientId().equals(proposedRevision.getClientId())) {
+ if (stamp.getUser() == null || stamp.getUser().equals(user)) {
--- End diff --
I think we need to be checking both the user and the client id here. To protect against the case when the same user has multiple clients open (like multiple browser windows). One client shouldn't be able to assume the transaction from another client. Should also have a unit test for this scenario.
---
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] nifi pull request: NIFI-1745: Refactor how revisions are handled a...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/454
---
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] nifi pull request: NIFI-1745: Refactor how revisions are handled a...
Posted by markap14 <gi...@git.apache.org>.
Github user markap14 commented on a diff in the pull request:
https://github.com/apache/nifi/pull/454#discussion_r64043396
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-optimistic-locking/src/main/java/org/apache/nifi/web/revision/NaiveRevisionManager.java ---
@@ -423,7 +440,7 @@ public boolean requestWriteLock(final Revision proposedRevision) throws ExpiredR
throw ise;
}
- if (stamp.getClientId() == null || stamp.getClientId().equals(proposedRevision.getClientId())) {
+ if (stamp.getUser() == null || stamp.getUser().equals(user)) {
--- End diff --
@mcgilman good call. I have addressed the issue and updated the PR.
---
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.
---