[sub]feat: add function events #714
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
guilhem-barthes
force-pushed
the
feat/add-function-events
branch
from
57404f2
to
cba53a2
Compare
FL-1160 Add function event
ContextStatuses:
SpecificationAcceptance criteria |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
guilhem-barthes
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
guilhem-barthes
commented
Aug 28, 2023
guilhem-barthes
commented
Aug 28, 2023
guilhem-barthes
commented
Aug 28, 2023
guilhem-barthes
commented
Aug 28, 2023
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
SdgJlbl
reviewed
Sep 18, 2023
backend/api/events/sync.py
Outdated
|
|
||
| if asset_type == failure_report_pb2.FAILED_ASSET_FUNCTION: | ||
| # Needed as this field is only in ComputeTask | ||
| compute_task_key = ComputeTask.objects.values_list("key", flat=True).get(function_id=asset_key) |
There was a problem hiding this comment.
I think we can have Model.MultipleObjectsReturned exception here, because several compute tasks can be linked to the same function.
There was a problem hiding this comment.
We probably should use rather iterate on the queryset (ensuring that it's not empty) to update all relevant CT status
There was a problem hiding this comment.
Filtering on status STATUS_DOING & STATUS_TODO
SdgJlbl
reviewed
Sep 18, 2023
| asset_class = ComputeTask | ||
|
|
||
| try: | ||
| asset = self.get_asset(request, report.key, channel_name, asset_class) |
There was a problem hiding this comment.
Shall we eventually (not now) refactor that to use PermissionMixin.download_file?
There was a problem hiding this comment.
yes we should !
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>
SdgJlbl
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Sep 19, 2023
## Companion PR - Substra/substra-backend#714 ## Description Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 ## How has this been tested? As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. ## Checklist - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com>
SdgJlbl
approved these changes
Sep 19, 2023
guilhem-barthes
added a commit
that referenced
this pull request
Oct 6, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
guilhem-barthes
added a commit
that referenced
this pull request
Oct 6, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
guilhem-barthes
added a commit
that referenced
this pull request
Oct 6, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
thbcmlowk
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Oct 11, 2023
- Substra/substra-backend#714 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com>
thbcmlowk
pushed a commit
that referenced
this pull request
Oct 11, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
thbcmlowk
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Oct 11, 2023
- Substra/substra-backend#714 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com>
thbcmlowk
pushed a commit
that referenced
this pull request
Oct 11, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
4 tasks
SdgJlbl
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Oct 20, 2023
- Substra/substra-backend#714 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com>
SdgJlbl
added a commit
that referenced
this pull request
Oct 20, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
SdgJlbl
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Oct 20, 2023
- Substra/substra-backend#714 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com>
thbcmlowk
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Oct 20, 2023
- Substra/substra-backend#714 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com>
thbcmlowk
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Oct 24, 2023
- Substra/substra-backend#714 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com>
thbcmlowk
pushed a commit
that referenced
this pull request
Oct 24, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
SdgJlbl
added a commit
that referenced
this pull request
Oct 25, 2023
* feat: decouple image builder from worker Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * fix: update skaffold config Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: add `ServiceAccount` and modify role Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: improve `wait_for_image_built` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: build image in new pod Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * chore: rename `deployment-builder.yaml` to `stateful-builder.yaml` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * chore: rename `stateful-builder.yaml` to `statefulset-builder.yaml` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * chore: centralize params Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: create `BuildTask` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: move some values to `builder` module Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: move more code to `builder` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: remove TaskProfiling as Celery task + save Entrypoint in DB Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * fix: extract entrypoint from registry Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * fix: make doc for helm chart Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * feat: build function at registration (#707) <!-- Please reference issue if any. --> <!-- Please include a summary of your changes. --> <!-- Please describe the tests that you ran to verify your changes. --> - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com> * feat: share images between backends (#708) Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * chore: update helm worklfow Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> * chore: add .DS_Store to gitignore Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> * chore: rm DS_Store Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> * chore: rm .DS_Store Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> * [sub]fix: add missing migration poc (#728) ## Description Add a migration missing in the poc. This migration alters two things: - modify `ComputeTaskFailureReport.logs` - modify `FunctionImage.file` This migration has been generated automatically with `make migrations` ## How has this been tested? <!-- Please describe the tests that you ran to verify your changes. --> ## Checklist - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * [sub]feat: add function events (#714) - Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com> * [sub]fix(app/orchestrator/resources): FunctionStatus.FUNCTION_STATUS_CREATED -> FunctionStatus.FUNCTION_STATUS_WAITING (#742) # Issue Backend FunctionStatus are not aligned with [orchestrator definitions](https://github.com/Substra/orchestrator/blob/poc-decoupled-builder/lib/asset/function.proto#L29-L36). In particular, `FunctionStatus.FUNCTION_STATUS_CREATED` leading to the following error: ```txt ValueError: 'FUNCTION_STATUS_WAITING' is not a valid FunctionStatus ``` ## Description FunctionStatus.FUNCTION_STATUS_CREATED -> FunctionStatus.FUNCTION_STATUS_WAITING ## How has this been tested? Running Camelyon benchmark on [poc-builder-flpc](https://substra.org-1.poc-builder-flpc.cg.owkin.tech/compute_plans/a420306f-5719-412b-ab9c-688b7bed9c70/tasks?page=1&ordering=-rank) environment. ## Checklist - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> * fix: builder using builder SA (#754) * fix: builder using builder SA Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * docs: changelog Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> --------- Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * fix: rebase changelog Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * fix: adapt to pydantic 2.x.x (#758) Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * [sub]fix(backend/image_transfert/encoder): update pydantic method (#763) * fix(backend/image_transfert/encoder): update pydantic method Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> * fix(backend/image_transfer/decoder): parse_raw -> model_validate_json Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> --------- Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> * [sub]chore: upgrade chart (#765) * chore(charts): bump chart version Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> * chore(charts/substra-backend/CHANGELOG): bring back unreleased section Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> --------- Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> * fix: post-rebase Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * chore: rationalize migrations Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * [sub]chore(builder): waitPostgresqlInitContainer (#764) * fix: builder using builder SA (#754) * fix: builder using builder SA Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * docs: changelog Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> --------- Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * chore(charts/substra-backend/templates/statefulset-builder): add init-container waitPostgresqlInitContainer Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> --------- Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> Co-authored-by: Guilhem Barthés <guilhem.barthes@owkin.com> --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com> Co-authored-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> Co-authored-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com>
SdgJlbl
pushed a commit
to Substra/orchestrator
that referenced
this pull request
Oct 25, 2023
## Details - Substra/substra-backend#714 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the `Function`. It also includes another PR (merged here), to have functions build logs working again. Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. ## Companion PR * orchestrator: #310 * backend: Substra/substra-backend#756 * frontend: Substra/substra-frontend#240 * substra-generator: owkin/substra-generator#131 ## Misc - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- ## Description <!-- Please reference issue if any. --> <!-- Please include a summary of your changes. --> ## How has this been tested? <!-- Please describe the tests that you ran to verify your changes. --> ## Checklist - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: thbcmlowk <thbcmlowk@users.noreply.github.com> Co-authored-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: guilhem-barthes <guilhem-barthes@users.noreply.github.com> Co-authored-by: thbcmlowk <thbcmlowk@users.noreply.github.com>
guilhem-barthes
added a commit
that referenced
this pull request
Oct 25, 2023
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
guilhem-barthes
added a commit
that referenced
this pull request
Feb 8, 2024
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
guilhem-barthes
added a commit
that referenced
this pull request
Feb 12, 2024
- Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com>
guilhem-barthes
added a commit
that referenced
this pull request
Feb 12, 2024
* feat: decouple image builder from worker Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * fix: update skaffold config Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: add `ServiceAccount` and modify role Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: build image in new pod Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * chore: rename `deployment-builder.yaml` to `stateful-builder.yaml` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * chore: rename `stateful-builder.yaml` to `statefulset-builder.yaml` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * chore: centralize params Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: create `BuildTask` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: move more code to `builder` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: remove TaskProfiling as Celery task + save Entrypoint in DB Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * feat: build function at registration (#707) <!-- Please reference issue if any. --> <!-- Please include a summary of your changes. --> <!-- Please describe the tests that you ran to verify your changes. --> - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com> * feat: share images between backends (#708) Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * chore: update helm worklfow Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> * [sub]fix: add missing migration poc (#728) ## Description Add a migration missing in the poc. This migration alters two things: - modify `ComputeTaskFailureReport.logs` - modify `FunctionImage.file` This migration has been generated automatically with `make migrations` ## How has this been tested? <!-- Please describe the tests that you ran to verify your changes. --> ## Checklist - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * [sub]feat: add function events (#714) - Substra/orchestrator#263 Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again. In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task() Fixes FL-1160 As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com> * [sub]fix(app/orchestrator/resources): FunctionStatus.FUNCTION_STATUS_CREATED -> FunctionStatus.FUNCTION_STATUS_WAITING (#742) # Issue Backend FunctionStatus are not aligned with [orchestrator definitions](https://github.com/Substra/orchestrator/blob/poc-decoupled-builder/lib/asset/function.proto#L29-L36). In particular, `FunctionStatus.FUNCTION_STATUS_CREATED` leading to the following error: ```txt ValueError: 'FUNCTION_STATUS_WAITING' is not a valid FunctionStatus ``` ## Description FunctionStatus.FUNCTION_STATUS_CREATED -> FunctionStatus.FUNCTION_STATUS_WAITING ## How has this been tested? Running Camelyon benchmark on [poc-builder-flpc](https://substra.org-1.poc-builder-flpc.cg.owkin.tech/compute_plans/a420306f-5719-412b-ab9c-688b7bed9c70/tasks?page=1&ordering=-rank) environment. ## Checklist - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> * fix: rebase changelog Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * feat: decouple image builder from worker Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> * feat: add `ServiceAccount` and modify role Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: build function at registration (#707) <!-- Please reference issue if any. --> <!-- Please include a summary of your changes. --> <!-- Please describe the tests that you ran to verify your changes. --> - [ ] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com> * feat: save status update in orc Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: use status for build waiting Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: re-add `container_image_exists` Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: rebase errors Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: format Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: tests Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: add `si` to building invokations Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: tests Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: apply feedback Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * fix: only import during typing Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * [sub]feat: modify computetask failure report (#727) ## Companion PR - Substra/orchestrator#277 - Substra/substra-frontend#240 ## Description The aim is to allow registering failure reports not only for compute task but for other kind of assets (for now, functions which are not building as part of the execution of a compute task) - Modifies `ComputeTaskFailureReport`: - renamed the model to `AssetFailureReport` - renamed field `compute_task_key` to `asset_key` (as we can now have a function key) - added field `asset_type` to provide - Updates protobuf reflecting the previous changes - refactor `download_file` in `PermissionMixin` to provide mroe flexibility (and decouple from DRF) - create new `FailableTask` (Celery task): - centralize the logic to submit asset failure ## How has this been tested? As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main. ## Checklist - [x] [changelog](../CHANGELOG.md) was updated with notable changes - [ ] documentation was updated --------- Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> * feat: add config to run celery in tests Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * feat: add tests Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * fix: remove rebqse duplicate Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * docs: changelog Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * fix: adapt to pydantic 2.x.x Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * fix: remove rebase artifacts Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> * fix: update to pydantic 2.x.x Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> --------- Signed-off-by: SdgJlbl <sarah.diot-girard@owkin.com> Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com> Signed-off-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com> Co-authored-by: SdgJlbl <sarah.diot-girard@owkin.com> Co-authored-by: ThibaultFy <50656860+ThibaultFy@users.noreply.github.com> Co-authored-by: Thibault Camalon <135698225+thbcmlowk@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Companion PR
Description
Add function events, used now we decoupled the building of the function with the execution of the compute task. For that it add a status field on the Function. It also includes another PR (merged here), to have functions build logs working again.
In a future PR, we will change the compute task execution to avoid having to wait_for_function_built in compute_task()
Fixes FL-1160
How has this been tested?
As this is going to be merged on a branch that is going to be merged to a POC branch, we use MNIST as a baseline of a working model. We will deal with failing tests on the POC before merging on main.
Checklist