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

Jump to bottom

feat!: update compute task workflow #365

Merged
merged 10 commits into from
Feb 13, 2024
Merged

feat!: update compute task workflow #365

merged 10 commits into from
Feb 13, 2024

Conversation

guilhem-barthes
Copy link
Collaborator

@guilhem-barthes guilhem-barthes commented Feb 6, 2024 •
edited

Companion PR

Description

Add function status in the compute task workflow. Now tasks are are going to have status TODO after the function is built.

Note: The status TODO will be changed and more statuses will be added in another PR.

Fixes FL-1204

How has this been tested?

CI 👇

Checklist

  • changelog was updated with notable changes
  • documentation was updated

@guilhem-barthes guilhem-barthes changed the title Feat/update compute task workflow feat: update compute task workflow Feb 6, 2024
@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow

@Owlfred
Copy link
Contributor

Owlfred commented Feb 7, 2024 •
edited

End to end tests: ❌ FAILURE

“Just Keep Swimming!” ― Dory, Finding Nemo

@guilhem-barthes guilhem-barthes force-pushed the feat/update-compute-task-workflow branch 2 times, most recently from 95d20bf to 509bb2f Compare February 7, 2024 14:55
@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=mnist

@Owlfred
Copy link
Contributor

Owlfred commented Feb 7, 2024 •
edited

End to end tests: ✔️ SUCCESS

@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow

@Owlfred
Copy link
Contributor

Owlfred commented Feb 7, 2024 •
edited

End to end tests: ✔️ SUCCESS

Crushed it!

@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=camelyon

@Owlfred
Copy link
Contributor

Owlfred commented Feb 8, 2024 •
edited

End to end tests: ✔️ SUCCESS

Awesome! 🎉

@guilhem-barthes guilhem-barthes mentioned this pull request Feb 8, 2024
Merged
2 tasks
@linear Linear
Copy link

linear bot commented Feb 8, 2024

FL-1204 Update compute task flow

@guilhem-barthes guilhem-barthes force-pushed the feat/update-compute-task-workflow branch from 1cc9174 to b37cc5c Compare February 8, 2024 10:58
@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=sdk,mnist

@Owlfred
Copy link
Contributor

Owlfred commented Feb 8, 2024 •
edited

End to end tests: ✔️ SUCCESS

You rock!

@guilhem-barthes guilhem-barthes marked this pull request as ready for review February 8, 2024 11:39
@guilhem-barthes guilhem-barthes requested a review from a team as a code owner February 8, 2024 11:39
@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=substrafl

@Owlfred
Copy link
Contributor

Owlfred commented Feb 12, 2024 •
edited

End to end tests: ❌ FAILURE

Jobs status:

“Just Keep Swimming!” ― Dory, Finding Nemo

SdgJlbl
SdgJlbl reviewed Feb 12, 2024
View reviewed changes
Copy link
Contributor

@SdgJlbl SdgJlbl left a comment

Choose a reason for hiding this comment

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

Thanks a lot for your work.
I feel like a few test cases are still missing (or at least, I'm unclear where the new behaviours are tested).
A few comments to update too :)

CHANGELOG.md Show resolved Hide resolved
lib/service/computetask_test.go Show resolved Hide resolved
lib/service/computetaskstate.go Outdated Show resolved Hide resolved
lib/service/computetaskstate.go Show resolved Hide resolved
lib/service/computetaskstate.go Outdated Show resolved Hide resolved
lib/service/computetaskstate_test.go Show resolved Hide resolved
lib/service/functionstate.go Show resolved Hide resolved
@guilhem-barthes guilhem-barthes changed the title feat: update compute task workflow feat!: update compute task workflow Feb 12, 2024
@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=sdk,mnist

@Owlfred
Copy link
Contributor

Owlfred commented Feb 12, 2024 •
edited

End to end tests: ❌ FAILURE

It'll stay between us, no one needs to know.

SdgJlbl
SdgJlbl approved these changes Feb 12, 2024
View reviewed changes
Copy link
Contributor

@SdgJlbl SdgJlbl left a comment

Choose a reason for hiding this comment

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

🚀

@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=sdk,mnist --always-build

@Owlfred
Copy link
Contributor

Owlfred commented Feb 12, 2024

Something happened while processing your message: error: unknown option '--always-build'

@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=sdk,mnist

@Owlfred
Copy link
Contributor

Owlfred commented Feb 12, 2024 •
edited

End to end tests: ❌ FAILURE

“I’m sorry, Dave. I’m afraid I can’t do that.” ― Hal, 2001: A Space Odyssey

@guilhem-barthes
chore: rename GetFunctionRunnableTasks to `GetFunctionFromTasksWith…
0199877 
…Status`

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
feat: add GetTasksbyFunction
9dafd10 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
chore: rename propagateDone to startChildrenTaskFromParents
286541b 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
feat: add CheckFunctionReady
b1fb7bf 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
feat: refactor logic when trask is finished to take in account task s…
1fe9c22 
…tatus

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
feat: add logic when function is ready
11cea97 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
fix: Add SetReadyFromWaiting to fix tests where tasks would never h…
f65cd2f 
…as status `TODO`

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
chore: fix linter issues
acd9992 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
docs: update changelog
5546559 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
chore: apply reviewer feedback
68cccf9 
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
@guilhem-barthes
Copy link
Collaborator Author

/e2e --refs substra-backend=feat/update-compute-task-workflow --tests=sdk,mnist

@Owlfred
Copy link
Contributor

Owlfred commented Feb 12, 2024 •
edited

End to end tests: ✔️ SUCCESS

“To infinity and beyond!” ― Buzz Lightyear, Toy Story

@guilhem-barthes guilhem-barthes merged commit 637103c into main Feb 13, 2024
6 checks passed
@guilhem-barthes guilhem-barthes deleted the feat/update-compute-task-workflow branch February 13, 2024 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SdgJlbl SdgJlbl SdgJlbl approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@guilhem-barthes @Owlfred @SdgJlbl