Runs and jobs can be created from pipeline version #2445
Conversation
… fetches pipelien version file.
|
/assign @IronPan |
|
/assign @IronPan |
backend/api/pipeline.proto
Outdated
| @@ -241,7 +248,7 @@ message Pipeline { | |||
| // version is used as default. (In the future, if desired by customers, we | |||
| // can allow them to set default version.) | |||
| // TODO(jingzhang36): expose this in API pipeline definition with FE changes. | |||
| if err != nil { | ||
| return nil, util.Wrap(err, "Unable to parse the parameter.") | ||
| } | ||
| resourceReferences, err := r.toModelResourceReferences(runId, common.Run, run.ResourceReferences) | ||
| resourceReferences, err := r.toModelResourceReferences( |
There was a problem hiding this comment.
nit - could we revert the fmt changes like this? the new code is arguably not as readable as the old format.
There was a problem hiding this comment.
This code snippet shows as "outdated" in my page view...probably page refresh thing and the good news is this line is indeed reverted.
| // (1) raw pipeline manifest in pipeline_spec | ||
| // (2) pipeline version in resource_references | ||
| var workflowSpecManifestBytes []byte | ||
| workflowSpecManifestBytes, err := |
There was a problem hiding this comment.
this line breaking is really not as readable as old version. please consider change the fmt style
backend/src/apiserver/server/util.go
Outdated
| @@ -241,3 +241,27 @@ func ValidatePipelineSpec(resourceManager *resource.ResourceManager, spec *api.P | |||
| } | |||
| return nil | |||
| } | |||
|
|
|||
| // Verify pipeline version in resource references as creator. | |||
| func VerifyPipelineVersionReferenceAsCreator(resourceManager *resource.ResourceManager, references []*api.ResourceReference) (*string, error) { | |||
There was a problem hiding this comment.
the name of this function is not very clear to me.
consider CheckPipelineVersionReference()
and put more description what is checked
- pipeline version reference exist
- it is a owner
- ID is a valid
- pipeline
There was a problem hiding this comment.
One question: since experiment is the owner of run, I set pipeline version as the creator. Let me know if you prefer it be an owner as well.
There was a problem hiding this comment.
sorry i mean it's a creator instead of a owner. i happened to use them interchagably.
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: IronPan 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 |
* re-generate api since #2445 changed api proto * Fix integration test for pipeline client * use generated constants
This change is