kind/feat: passing artifacts between tasks #7978
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
|
Skipping CI for Draft Pull Request. |
tekton-robot
added
do-not-merge/work-in-progress
|
The following is the coverage report on the affected files.
|
tekton-robot
removed
the
do-not-merge/work-in-progress
|
The following is the coverage report on the affected files.
|
tekton-robot
added
needs-rebase
ericzzzzzzz
force-pushed
the
passing-artifacts-between-tasks
branch
from
00599d0
to
cd0b12d
Compare
tekton-robot
removed
the
needs-rebase
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
tekton-robot
added
the
do-not-merge/work-in-progress
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
tekton-robot
removed
the
size/XL
|
The following is the coverage report on the affected files.
|
ericzzzzzzz
force-pushed
the
passing-artifacts-between-tasks
branch
from
46419a7
to
34c3828
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
Hi @vdemeester , I removed the syntax to get default artifact, please take a look when you have time. Thanks |
Just a ping @vdemeester when you have a chance. Thanks! |
vdemeester
reviewed
Jul 3, 2024
| for _, vv := range v.Values { | ||
| exists := false | ||
| for _, av := range outputMap[v.Name] { | ||
| if cmp.Equal(vv, av) { |
There was a problem hiding this comment.
Any reason to use cmp.Equal here ?
There was a problem hiding this comment.
vv, av are instances of ArtifactValue struct, this struct contains a map, we so cannot use == operator to compare them, that's why cmp.Equal is used here.
type ArtifactValue struct {
Digest map[Algorithm]string `json:"digest,omitempty"` // Algorithm-specific digests for verifying the content (e.g., SHA256)
Uri string `json:"uri,omitempty"` // Location where the artifact value can be retrieved
}| if e.ArtifactsDirectory != "" { | ||
| artifactsDir = e.ArtifactsDirectory | ||
| } | ||
| fp = filepath.Join(artifactsDir, "provenance.json") |
There was a problem hiding this comment.
We might want to make this an exported constant at least. Also, naive question, why the name provenance.json ?
There was a problem hiding this comment.
agreed about exported const.
provenance.json because it contains information about the location of the artifacts (i.e. Uri and digest). We can certainly change the name to something else. We thought that it was accurate.
There was a problem hiding this comment.
To make fp a constant value, it would require artifactsDir to be a constant first, but it needs to be a variable as we need to overwrite this value for test purpose. This is the same pattern used for resultDir
pipeline/pkg/entrypoint/entrypointer.go
Lines 267 to 273 in 34c3828
ericzzzzzzz
force-pushed
the
passing-artifacts-between-tasks
branch
from
34c3828
to
6ff6e9b
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
ericzzzzzzz
force-pushed
the
passing-artifacts-between-tasks
branch
from
6ff6e9b
to
d2b4341
Compare
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
/retest |
|
/approve |
tekton-robot
added
the
approved
tekton-robot
added
the
needs-rebase
vdemeester
approved these changes
Jul 5, 2024
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chitrangpatel, vdemeester 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 |
ericzzzzzzz
force-pushed
the
passing-artifacts-between-tasks
branch
from
d2b4341
to
1e5fb7f
Compare
tekton-robot
removed
the
needs-rebase
|
The following is the coverage report on the affected files.
|
|
The following is the coverage report on the affected files.
|
|
/lgtm |
|
/retest |
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.
fixes: #7870
Changes
Support passing artifacts between tasks as described in this tep
Task author can write task artifacts to
$(artifacts.path)Tasks can consume artifacts from another tasks with similar syntax introduced for step Artifacts
$(tasks.<task-name>.outputs.<artifacts-category-name>), ,$(tasks.<task-name>.inputs.<artifacts-category-name>)Note that both stepArtifacts which are written to
$(step.artifacts.path)and task artifacts which are written to$(artifacts.path)are aggregated into TaskRun.Status.Artifacts with deduplication.Submitter Checklist
As the author of this PR, please check off the items in this checklist:
/kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tepRelease Notes