feat(tekton-kfptask): Update kfptask to publish completed dag status #1426
Conversation
|
/cc @yhwang This is a POC design and I will try to propose something more complete to the KFP community's driver package when we have a more complete design. |
| status := pb.Execution_COMPLETE | ||
| execution, err := mlmd.GetExecution(ctx, opts.DAGExecutionID) | ||
| if err = mlmd.PublishExecution(ctx, execution, outputParameters, nil, status); err != nil { | ||
| return fmt.Errorf("failed to publish: %w", err) |
There was a problem hiding this comment.
no need to construct a new error here, because of the defer
There was a problem hiding this comment.
The defer statement is the high level debugging for this function, the fmt error is just to help print out more specific error statement.
There was a problem hiding this comment.
nope, this error gets overridden by the deffer. there is no meaning to construct a new error here.
I got it. I thought there is only one type of error here. NVM
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Tomcli, yhwang The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Which issue is resolved by this Pull Request:
Resolves #
Description of your changes:
Our current publisher only being executed when the dag is completed. Thus it make sense to also update mlmd status as complete for that dag.
For failing status, caching entry, and dag output artifacts, it only make sense to implement it after we merged the publisher as part of the sub-dag reconciliation logic. Otherwise any failed sub-dag will never run the publisher task and the cache entry and output artifacts info has different logic when it became part of the sub-dag reconciliation logic.
Environment tested:
python --version):tkn version):kubectl version):/etc/os-release):Checklist: