Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add reference name to resource reference API proto #1781

Merged
merged 20 commits into from
Aug 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 38 additions & 29 deletions backend/api/go_client/resource_reference.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions backend/api/resource_reference.proto
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ message ResourceKey {
message ResourceReference {
ResourceKey key = 1;

// The name of the resource that referred to.
string name = 3;

// Required field. The relationship from referred resource to the object.
Relationship relationship = 2;
}
4 changes: 4 additions & 0 deletions backend/api/swagger/job.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions backend/api/swagger/kfp_api_single_file.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions backend/api/swagger/run.swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions backend/src/apiserver/model/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ type Experiment struct {
CreatedAtInSec int64 `gorm:"column:CreatedAtInSec; not null"`
}

func (r Experiment) GetValueOfPrimaryKey() string {
return r.UUID
func (e Experiment) GetValueOfPrimaryKey() string {
return e.UUID
}

func GetExperimentTablePrimaryKeyColumn() string {
Expand Down
5 changes: 4 additions & 1 deletion backend/src/apiserver/model/resource_reference.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,15 @@ type ResourceReference struct {
// The ID of the resource that been referenced to.
ReferenceUUID string `gorm:"column:ReferenceUUID; not null; "`

// The name of the resource that been referenced to.
ReferenceName string `gorm:"column:ReferenceName; not null; "`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new required field usually breaks backwards compatibility with all existing clients.
What do you think about keeping it optional?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is database schema requirement which won't be surfaced through UI. Please see integration tests for the sample request.


// The type of the resource that been referenced to.
ReferenceType common.ResourceType `gorm:"column:ReferenceType; not null; primary_key"`

// The relationship between the resource object and the resource that been referenced to.
Relationship common.Relationship `gorm:"column:Relationship; not null; "`

// The json formatted blob of the resource reference.
Payload string `gorm:"column:Payload; not null; "`
Payload string `gorm:"column:Payload; not null; size:65535 "`
}
50 changes: 43 additions & 7 deletions backend/src/apiserver/resource/model_converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import (
"github.com/kubeflow/pipelines/backend/src/common/util"
)

func ToModelRunMetric(metric *api.RunMetric, runUUID string) *model.RunMetric {
func (r *ResourceManager) ToModelRunMetric(metric *api.RunMetric, runUUID string) *model.RunMetric {
return &model.RunMetric{
RunUUID: runUUID,
Name: metric.GetName(),
Expand All @@ -36,12 +36,12 @@ func ToModelRunMetric(metric *api.RunMetric, runUUID string) *model.RunMetric {

// The input run might not contain workflowSpecManifest, but instead a pipeline ID.
// The caller would retrieve workflowSpecManifest and pass in.
func ToModelRunDetail(run *api.Run, runId string, workflow *util.Workflow, workflowSpecManifest string) (*model.RunDetail, error) {
func (r *ResourceManager) ToModelRunDetail(run *api.Run, runId string, workflow *util.Workflow, workflowSpecManifest string) (*model.RunDetail, error) {
params, err := toModelParameters(run.PipelineSpec.Parameters)
if err != nil {
return nil, util.Wrap(err, "Unable to parse the parameter.")
}
resourceReferences, err := toModelResourceReferences(runId, common.Run, run.ResourceReferences)
resourceReferences, err := r.toModelResourceReferences(runId, common.Run, run.ResourceReferences)
if err != nil {
return nil, util.Wrap(err, "Unable to convert resource references.")
}
Expand All @@ -67,12 +67,12 @@ func ToModelRunDetail(run *api.Run, runId string, workflow *util.Workflow, workf
}, nil
}

func ToModelJob(job *api.Job, swf *util.ScheduledWorkflow, workflowSpecManifest string) (*model.Job, error) {
func (r *ResourceManager) ToModelJob(job *api.Job, swf *util.ScheduledWorkflow, workflowSpecManifest string) (*model.Job, error) {
params, err := toModelParameters(job.PipelineSpec.Parameters)
if err != nil {
return nil, util.Wrap(err, "Error parsing the input job.")
}
resourceReferences, err := toModelResourceReferences(string(swf.UID), common.Job, job.ResourceReferences)
resourceReferences, err := r.toModelResourceReferences(string(swf.UID), common.Job, job.ResourceReferences)
if err != nil {
return nil, util.Wrap(err, "Error to convert resource references.")
}
Expand Down Expand Up @@ -145,8 +145,8 @@ func toModelParameters(apiParams []*api.Parameter) (string, error) {
return string(paramsBytes), nil
}

func toModelResourceReferences(
resourceId string, resourceType common.ResourceType, apiRefs []*api.ResourceReference) ([]*model.ResourceReference, error) {
func (r *ResourceManager) toModelResourceReferences(
resourceId string, resourceType common.ResourceType, apiRefs []*api.ResourceReference) ([]*model.ResourceReference, error) {
var modelRefs []*model.ResourceReference
for _, apiRef := range apiRefs {
modelReferenceType, err := common.ToModelResourceType(apiRef.Key.Type)
Expand All @@ -157,14 +157,50 @@ func toModelResourceReferences(
if err != nil {
return nil, util.Wrap(err, "Failed to convert relationship")
}
referenceName, err := r.getResourceName(modelReferenceType, apiRef.Key.Id)
if err != nil {
return nil, util.Wrap(err, "Failed to find the referred resource")
}
modelRef := &model.ResourceReference{
ResourceUUID: resourceId,
ResourceType: resourceType,
ReferenceUUID: apiRef.Key.Id,
ReferenceName: referenceName,
ReferenceType: modelReferenceType,
Relationship: modelRelationship,
}
modelRefs = append(modelRefs, modelRef)
}
return modelRefs, nil
}

func (r *ResourceManager) getResourceName(resourceType common.ResourceType, resourceId string) (string, error) {
switch resourceType {
case common.Experiment:
experiment, err := r.GetExperiment(resourceId)
if err != nil {
return "", util.Wrap(err, "Referred experiment not found.")
}
return experiment.Name, nil
case common.Pipeline:
pipeline, err := r.GetPipeline(resourceId)
if err != nil {
return "", util.Wrap(err, "Referred pipeline not found.")
}
return pipeline.Name, nil
case common.Job:
job, err := r.GetJob(resourceId)
if err != nil {
return "", util.Wrap(err, "Referred job not found.")
}
return job.DisplayName, nil
case common.Run:
run, err := r.GetRun(resourceId)
if err != nil {
return "", util.Wrap(err, "Referred run not found.")
}
return run.DisplayName, nil
default:
return "", util.NewInvalidInputError("Unsupported resource type: %s", string(resourceType))
}
}
Loading