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.

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

Manage upload to ipfs #230

Merged
merged 14 commits into from
Mar 11, 2019
Merged

Manage upload to ipfs #230

merged 14 commits into from
Mar 11, 2019

Conversation

Ugo
Copy link

@Ugo Ugo commented Mar 11, 2019

The idea here is to publish the results depending on the beneficiary of the task:

  • if the beneficiary is null (meaning the result is public), the result will be pushed to the ipfs node defined in the core.
  • if the beneficiary is not null, the result will be pushed in the mongo repo of the result repo (same as before)

Some refactoring will be needed to split the replicate update status into several entrypoints instead of a single one at the moment.

@Ugo Ugo requested review from zguesmi and jeremyjams March 11, 2019 12:36
@Ugo Ugo changed the title Ipfs Manage upload to ipfs Mar 11, 2019
Copy link
Member

@jeremyjams jeremyjams left a comment

Choose a reason for hiding this comment

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

Great one, just tiny issues :)

@@ -346,10 +360,24 @@ private boolean isTaskStatusFailOnChain(String chainTaskId, String walletAddress
}
}

boolean isPublicResult(String chainTaskId, Integer chainId) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move isPublicResult to common? (We're gonna need it for the seperate Result Repo)

Copy link
Author

Choose a reason for hiding this comment

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

right, moved to common.

boolean isPublicResult(String chainTaskId) {
return isPublicResult(chainTaskId, 0);
}

boolean isPublicResult(String chainTaskId, Integer chainId) {
Copy link
Member

Choose a reason for hiding this comment

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

We have a duplicate here

Copy link
Author

Choose a reason for hiding this comment

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

removed.

return Optional.empty();
}

public boolean doesContentExists(String ipfsHash) {
Copy link
Member

Choose a reason for hiding this comment

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

doesContentExist* (no 's')!

Copy link
Author

Choose a reason for hiding this comment

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

thanks, changed.

@@ -170,6 +169,16 @@ private int getNbReplicatesWithGivenStatusJustBeforeWorkerLost(String chainTaskI
return Optional.empty();
}

public Optional<Replicate> getReplicateWithResultUploadedStatus(String chainTaskId) {
Copy link
Member

Choose a reason for hiding this comment

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

It seems we have an issue when worker is lost after UPLOADED

Copy link
Member

Choose a reason for hiding this comment

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

private int getNbReplicatesWithGivenStatusJustBeforeWorkerLost(String chainTaskId, ReplicateStatus status) {

Copy link
Author

Choose a reason for hiding this comment

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

great catch, thanks! I've modified the code.

Copy link
Member

@jeremyjams jeremyjams left a comment

Choose a reason for hiding this comment

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

Great, thanks for the changes!

@Ugo Ugo merged commit c5ee0c6 into master Mar 11, 2019
@Ugo Ugo deleted the ipfs branch March 11, 2019 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants