feat: add function event #263

guilhem-barthes · 2023-08-21T14:46:03Z

Companion PR

[sub]feat: add function events 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

changelog was updated with notable changes
documentation was updated

linear · 2023-08-21T14:46:05Z

FL-1160 Add function event

Context

Tranfer these statuses through the orchestrator.

Change

Specification

Acceptance criteria

guilhem-barthes · 2023-08-21T14:47:23Z

/e2e

Owlfred · 2023-08-21T14:47:25Z

End to end tests: ✔️ SUCCESS

lib/asset/sql.go

lib/service/functionstate.go

server/distributed/adapters/function.go

chaincode/ledger/dbal_function.go

lib/asset/function.proto

server/standalone/migration/000058_add-function-status.up.sql

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: Guilhem Barthés <guilhem.barthes@owkin.com>

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

lib/service/functionstate.go

mblottiere

Review still in progress

lib/asset/failure_report.proto

lib/asset/failure_report_validation.go

mblottiere · 2023-09-13T16:00:43Z

lib/asset/function.proto

@@ -18,6 +18,23 @@ message FunctionOutput {
  bool multiple = 2;
 }

+enum FunctionAction {
+  FUNCTION_ACTION_UNKNOWN = 0;
+  FUNCTION_ACTION_BUILDING = 1;


Nitpick: an action would probably be a verb, something like FUNCTION_ACTION_BUILD / Cancel / Fail / Ready
Also, make sure you are consistent (for instance canceling/canceled/cancel) for all the entries.

Edit, after reading other parts of the code: actions should match their statuses:

build -> building

cancel -> canceled

fail -> failed
ready is not clear, does it mean "built"? then build -> built
Created can be confusing, one would expect that it's usable, but actually we want "built". Maybe "pending" or "waiting" would be more explicit.

We decided to rename CREATED to WAITING, but we kept READY as we envision use-cases where we do not build internally in Substra

e2e/client/client.go

lib/service/computetaskstate_test.go

lib/service/functionstate.go

mblottiere · 2023-09-13T16:12:17Z

lib/service/functionstate.go

+)
+
+// functionStateEvents is the definition of the state machine representing function states
+var functionStateEvents = fsm.Events{


Some doc in docs/assets/function.md to graph the state machine would be super helpful.

lib/service/functionstate.go

lib/service/functionstate_test.go

mblottiere · 2023-09-14T12:24:38Z

server/standalone/dbal/function.go

@@ -198,7 +200,7 @@ func (d *DBAL) queryFunctions(p *common.Pagination, filter *asset.FunctionQueryF
 	return functions, bookmark, nil
 }

-// UpdateFunction updates the mutable fields of an function in the DB. List of mutable fields: name.
+// UpdateFunction updates the mutable fields of an function in the DB. List of mutable fields: name, status.
 func (d *DBAL) UpdateFunction(function *asset.Function) error {


Then that should be UpdateFunctionName since it only deals with name.
Maybe double check the API: do you need 2 distinct functions here?

mblottiere · 2023-09-14T12:25:16Z

server/standalone/dbal/function.go

@@ -207,3 +209,12 @@ func (d *DBAL) UpdateFunction(function *asset.Function) error {

 	return d.exec(stmt)
 }
+
+func (d *DBAL) UpdateFunctionStatus(functionKey string, status asset.FunctionStatus) error {


If you keep this one, maybe worth adding a test case

Removed and merged call with previous UpdateFunction

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

## Companion PR - Substra/orchestrator#263 ## 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 - [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>

- 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>

- 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>

- 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>

- 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>

- 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>

- 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>

- 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>

- 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>

- 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>

* 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)    - [ ] [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?  ## 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>

- 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>

* 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)    - [ ] [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?  ## 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)    - [ ] [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>

guilhem-barthes changed the title ~~Feat/add function event~~ feat: add function event Aug 21, 2023

guilhem-barthes commented Aug 28, 2023

View reviewed changes

lib/asset/sql.go Outdated Show resolved Hide resolved

guilhem-barthes commented Aug 28, 2023

View reviewed changes

lib/service/functionstate.go Outdated Show resolved Hide resolved

guilhem-barthes commented Aug 28, 2023

View reviewed changes

server/distributed/adapters/function.go Outdated Show resolved Hide resolved

guilhem-barthes commented Aug 28, 2023

View reviewed changes

chaincode/ledger/dbal_function.go Outdated Show resolved Hide resolved

SdgJlbl reviewed Aug 30, 2023

View reviewed changes

lib/asset/function.proto Outdated Show resolved Hide resolved

SdgJlbl reviewed Aug 30, 2023

View reviewed changes

server/standalone/migration/000058_add-function-status.up.sql Outdated Show resolved Hide resolved

guilhem-barthes mentioned this pull request Sep 7, 2023

[sub]feat: add function events Substra/substra-backend#714

Merged

2 tasks

guilhem-barthes changed the base branch from main to poc-decoupled-builder September 13, 2023 13:45

guilhem-barthes and others added 18 commits September 13, 2023 16:40

feat: add function status

6af6ef1

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

feat: add UpdateFunctionStatus

86ffe14

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

[auto] generate database diagram

b52e103

Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: format

5274e26

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

feat: add function state machine for state

553e024

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

feat: Propagate fail when function fail

f2740c7

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: add adapters for ApplyFunctionAction

298aff2

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: re-add TestConcurrency

ae71247

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: format

84556f2

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: add err check in ApplyFunctionAction

542979e

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: logger in FunctionService

8e38883

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: return type in function onFailure

3627c35

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: format

f0df0aa

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: apply review feedback

67e544d

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

feat: add GetFunctionRunnableTasksKeys

70d531d

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: format

0117385

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: add logging on propagateFunctionCancelation error

1af7766

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: add FUNCTION_STATUS_UNKOWN

9b72fd4

Signed-off-by: Guilhem Barthes <guilhem.barthes@owkin.com> Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

AlexandrePicosson reviewed Sep 13, 2023

View reviewed changes

lib/service/functionstate.go Outdated Show resolved Hide resolved

mblottiere reviewed Sep 13, 2023

View reviewed changes

mblottiere reviewed Sep 14, 2023

View reviewed changes

lib/service/functionstate_test.go Show resolved Hide resolved

mblottiere reviewed Sep 14, 2023

View reviewed changes

guilhem-barthes added 4 commits September 18, 2023 18:10

chore: Rename FUNCTION_STATUS_CREATED to FUNCTION_STATUS_WAITING

3eee1e4

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

chore: remove UpdateTaskStatus

9601dae

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

docs: improve comments

202824f

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

fix: apply feedback

274952f

Signed-off-by: Guilhem Barthés <guilhem.barthes@owkin.com>

SdgJlbl approved these changes Sep 19, 2023

View reviewed changes

SdgJlbl merged commit e24edd7 into poc-decoupled-builder Sep 19, 2023
4 of 5 checks passed

SdgJlbl deleted the feat/add-function-event branch September 19, 2023 07:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add function event #263

feat: add function event #263

guilhem-barthes commented Aug 21, 2023 •

edited

Loading

linear bot commented Aug 21, 2023 •

edited by guilhem-barthes

Loading

Context

Specification

Acceptance criteria

guilhem-barthes commented Aug 21, 2023

Owlfred commented Aug 21, 2023 •

edited

Loading

mblottiere left a comment

mblottiere Sep 13, 2023

guilhem-barthes Sep 18, 2023

mblottiere Sep 13, 2023

mblottiere Sep 14, 2023

mblottiere Sep 14, 2023

guilhem-barthes Sep 18, 2023

feat: add function event #263

feat: add function event #263

Conversation

guilhem-barthes commented Aug 21, 2023 • edited Loading

Companion PR

Description

How has this been tested?

Checklist

linear bot commented Aug 21, 2023 • edited by guilhem-barthes Loading

Context

Specification

Acceptance criteria

guilhem-barthes commented Aug 21, 2023

Owlfred commented Aug 21, 2023 • edited Loading

mblottiere left a comment

Choose a reason for hiding this comment

mblottiere Sep 13, 2023

Choose a reason for hiding this comment

guilhem-barthes Sep 18, 2023

Choose a reason for hiding this comment

mblottiere Sep 13, 2023

Choose a reason for hiding this comment

mblottiere Sep 14, 2023

Choose a reason for hiding this comment

mblottiere Sep 14, 2023

Choose a reason for hiding this comment

guilhem-barthes Sep 18, 2023

Choose a reason for hiding this comment

guilhem-barthes commented Aug 21, 2023 •

edited

Loading

linear bot commented Aug 21, 2023 •

edited by guilhem-barthes

Loading

Owlfred commented Aug 21, 2023 •

edited

Loading