Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ContributeAndFinalized replicate statuses #408

Merged
merged 10 commits into from
May 3, 2023
Merged

Add ContributeAndFinalized replicate statuses #408

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8afa774
Add `CONTRIBUTE_AND_FINALIZED` replicate statuses
mcornaton Apr 25, 2023
b73dd19
Add `@Deprecated` on `ReplicateStatus` methods
mcornaton Apr 25, 2023
0fa95b0
Update `CHANGELOG.md`
mcornaton Apr 25, 2023
27d6b82
Add `@deprecated` Javadoc annotation
mcornaton Apr 25, 2023
2c4b5f8
Remove `@Deprecated` annotations
mcornaton Apr 27, 2023
13117c8
Merge branch 'develop' into feature/contribute-and-finalize-replicate…
mcornaton Apr 27, 2023
c461795
Swap `CONTRIBUTE_AND_FINALIZE_DONE` and `CONTRIBUTE_AND_FINALIZE_FAIL…
mcornaton May 2, 2023
258f8ae
Remove `CONTRIBUTE_AND_FINALIZE_DONE` from `getFailableStatuses`
mcornaton May 2, 2023
62b1bd3
Remove `mavenLocal` accidental addition from repositories
mcornaton May 2, 2023
b585b98
Remove `CONTRIBUTE_AND_FINALIZE_ONGOING` from `getCompletableStatuses`
mcornaton May 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.

## [[NEXT]](https://github.com/iExecBlockchainComputing/iexec-common/releases/tag/vNEXT) 2023

### New Features
- Add `CONTRIBUTE_AND_FINALIZE` replicate statuses. (#408)
### Bug Fixes
- Log `computedFilePath` instead of `computedFileDir`. (#409)

## [[8.0.0]](https://github.com/iExecBlockchainComputing/iexec-common/releases/tag/v8.0.0) 2023-04-12

### Quality
Expand Down
20 changes: 19 additions & 1 deletion src/main/java/com/iexec/common/replicate/ReplicateStatus.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ public enum ReplicateStatus {
RESULT_UPLOADING,
RESULT_UPLOAD_FAILED,
RESULT_UPLOADED,
CONTRIBUTE_AND_FINALIZE_ONGOING,
CONTRIBUTE_AND_FINALIZE_DONE,
CONTRIBUTE_AND_FINALIZE_FAILED,
COMPLETING,
COMPLETE_FAILED,
COMPLETED,
Expand Down Expand Up @@ -102,6 +105,8 @@ public static List<ReplicateStatus> getSuccessStatuses() {
CONTRIBUTED,
REVEALING,
REVEALED,
CONTRIBUTE_AND_FINALIZE_ONGOING,
CONTRIBUTE_AND_FINALIZE_DONE,
COMPLETING,
COMPLETED));
}
Expand All @@ -116,6 +121,7 @@ public static List<ReplicateStatus> getFailureStatuses() {
REVEAL_FAILED,
RESULT_UPLOAD_REQUEST_FAILED,
RESULT_UPLOAD_FAILED,
CONTRIBUTE_AND_FINALIZE_FAILED,
COMPLETE_FAILED,
FAILED);
}
Expand All @@ -132,6 +138,8 @@ public static List<ReplicateStatus> getCompletableStatuses() {
RESULT_UPLOADING,
RESULT_UPLOAD_FAILED,
RESULT_UPLOADED,
CONTRIBUTE_AND_FINALIZE_ONGOING,
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we sure as REVEALING is not part of this list ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As far as I understand this part, this list should return all "non-uncompletable statuses", which is not the case right now. Also, it is used only for tests in iexec-core.
Let's rework the tests in iexec-core so that we can remove this method.

Copy link
Contributor

Choose a reason for hiding this comment

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

Honestly, I would go for the consistency in a first step and to go for the cleanup in a second one.

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure about your "non-uncompletable" ?
For me it is the list of states allowing to go to COMPLETING state which are

  • REVEALED (worker part of the consensus)
  • CONTRIBUTE_AND_FINALIZE_DONE (worker has revealed and has uploaded its results)
  • RESULT_UPLOAD something as the worker is part of the consensus and if this worker fails, to upload, another one could do it (to check if it is really supported)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please have a look at the only use of this method to understand what I mean:
https://github.com/iExecBlockchainComputing/iexec-core/blob/1e6d93d197b0346c4b46a62278d5b7286e56dd0f/src/test/java/com/iexec/core/replicate/ReplicateListenersTests.java#L164-L175

Copy link
Contributor Author

Choose a reason for hiding this comment

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

After some discussions, we've decided to remove CONTRIBUTE_AND_FINALIZE_ONGOING from this list.
Please see b585b98.

CONTRIBUTE_AND_FINALIZE_DONE,
COMPLETING,
COMPLETE_FAILED);
}
Expand Down Expand Up @@ -160,6 +168,9 @@ public static List<ReplicateStatus> getFailableStatuses() {
CONTRIBUTED,
REVEALING,
REVEAL_FAILED,
CONTRIBUTE_AND_FINALIZE_ONGOING,
CONTRIBUTE_AND_FINALIZE_DONE,
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure about keeping CONTRIBUTE_AND_FINALIZE_DONE here as the REVEALED state is not present in this list.
When CONTRIBUTE_AND_FINALIZE_DONE, result has been CONTRIBUTED and PROVED (aka REVEALED) on chain. I am not sure the task can be failed in this case.
I would rather comment this and ask a question to our experts when they are back.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do agree with your argument. Thanks for pointing it out!
Please see 258f8ae.

CONTRIBUTE_AND_FINALIZE_FAILED,
ABORTED);
}

Expand Down Expand Up @@ -198,6 +209,9 @@ public static List<ReplicateStatus> getRecoverableStatuses() {
RESULT_UPLOADING,
RESULT_UPLOAD_FAILED, // can complete later
RESULT_UPLOADED,
CONTRIBUTE_AND_FINALIZE_ONGOING,
CONTRIBUTE_AND_FINALIZE_DONE,
CONTRIBUTE_AND_FINALIZE_FAILED,
COMPLETING,
RECOVERING);
}
Expand Down Expand Up @@ -249,6 +263,9 @@ public static List<ReplicateStatus> getNonFinalWorkflowStatuses() {
RESULT_UPLOADING,
RESULT_UPLOAD_FAILED,
RESULT_UPLOADED,
CONTRIBUTE_AND_FINALIZE_ONGOING,
CONTRIBUTE_AND_FINALIZE_DONE,
CONTRIBUTE_AND_FINALIZE_FAILED,
jbern0rd marked this conversation as resolved.
Show resolved Hide resolved
COMPLETING,
COMPLETE_FAILED);
}
Expand Down Expand Up @@ -311,7 +328,8 @@ public static List<ReplicateStatus> getSuccessStatusesBeforeComputed() {
public static List<ReplicateStatus> getUncompletableStatuses() {
return Arrays.asList(
CONTRIBUTE_FAILED,
REVEAL_FAILED
REVEAL_FAILED,
CONTRIBUTE_AND_FINALIZE_FAILED
//RESULT_UPLOAD_REQUEST_FAILED, // still good if don't upload
//RESULT_UPLOAD_FAILED, //still good if don't upload
);
Expand Down