Skip to content
This repository has been archived by the owner on Jun 19, 2024. It is now read-only.

Fix #1715: ApplyService#applyProjectRequest should be (truly) idempotent #1717

Merged
merged 1 commit into from
Oct 4, 2019

Conversation

rohanKanojia
Copy link
Member

Fix #1715

@@ -1071,6 +1075,9 @@ public boolean applyProject(Project project) {
* Returns true if the ProjectRequest is created
*/
public boolean applyProjectRequest(ProjectRequest entity) {
// Check whether project creation attempted before
if (projectsCreated.contains(getName(entity)))
return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the method signature, I think the return value should be false if projectsCreated contains getName(entity)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Thanks 👍

Copy link
Contributor

@devang-gaur devang-gaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a small change. We need curly brackets everywhere for clarity.

@@ -1071,6 +1075,9 @@ public boolean applyProject(Project project) {
* Returns true if the ProjectRequest is created
*/
public boolean applyProjectRequest(ProjectRequest entity) {
// Check whether project creation attempted before
if (projectsCreated.contains(getName(entity)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rohanKanojia curly brackets, please!

@rohanKanojia rohanKanojia merged commit 56d96f5 into fabric8io:master Oct 4, 2019
@rohanKanojia rohanKanojia added the jkube/pending The issue/PR has to be taken care of in JKube https://github.com/eclipse/jkube label Oct 24, 2019
rohanKanojia added a commit to rohanKanojia/jkube that referenced this pull request Nov 7, 2019
@rohanKanojia rohanKanojia removed the jkube/pending The issue/PR has to be taken care of in JKube https://github.com/eclipse/jkube label Nov 7, 2019
rohanKanojia added a commit to eclipse-jkube/jkube that referenced this pull request Nov 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ApplyService#applyProjectRequest should be (truly) idempotent
3 participants