feat(sdk): CLI - Allow upload_version by the pipeline's name. Fixes #3901 #4087

NikeNano · 2020-06-26T08:53:35Z

Description of your changes:
This pr updates the KFP cli to allow pipeline upload_version to accept name instead of id. This will make is easier for some users and allow for a larger flexibility.

This should fix PR: #3901

We could also the check that only one/and always one of pipeline_name and pipeline_is using click, whould this be prefered?

kubeflow-bot · 2020-06-26T08:53:42Z

This change is

sdk/python/kfp/_client.py

NikeNano · 2020-06-26T14:04:30Z

Thanks for the feedback @Bobgy. I have not used protobuff before but gave it a try and got it working at least :)! I am not sure where it would make the most sense to put filter_pb2.py and if it should be auto generated and if so how? Concerning the filtering on namespace I am not sure how we can do it? Is it allowed to do the same way?

sdk/python/kfp/cli/pipeline.py

NikeNano · 2020-06-30T15:06:19Z

Thanks for the feedback @Ark-kun, resolved the comments.

NikeNano · 2020-07-01T08:34:28Z

/test kubeflow-pipeline-sample-test

sdk/python/kfp/_client.py

NikeNano · 2020-07-01T16:15:29Z

@Ark-kun I have updated but are getting some issues with ProtoBuf and it seems to be a issue related to versions for ProtoBuf. I have updated the packages in the requirement file but are builds/test using dependencies from somewhere else?

This threads talks about the issue: https://stackoverflow.com/questions/61922334/how-to-solve-attributeerror-module-google-protobuf-descriptor-has-no-attribu

Bobgy

Great work! Thanks for figuring out how to generate a proper helper class for the filter!

sdk/python/kfp/_client.py

sdk/python/kfp/filter_pb2.py

Bobgy · 2020-07-03T03:01:59Z

updated title to better comply with our convention

Bobgy

thanks @NikeNano!

mostly good now. I added comments about some implementation details

sdk/python/kfp/cli/pipeline.py

sdk/python/kfp/_client.py

Bobgy · 2020-07-06T08:16:51Z

sdk/python/kfp/_client.py

+    Args:
+      name: pipeline name
+      page_token: token for starting of the page.
+      page_size: size of the page.


reminder, also the docstring

sorry, multi tasking while listening in to some meetings.

Bobgy · 2020-07-06T08:18:42Z

Backup plan: did you consider constructing the json payload by yourself, the filter package is just a helper, we may not need it at all like in frontend code sample.

I though about it but, assumed the preferred way was to built the proper helper file. I will push an solution where we do it as the frontend :)

I agree proper helper is the preferred way, just that when it's hard to get it working. We can choose the practical path.

Bobgy · 2020-07-06T08:20:15Z

Updated title to be more concise and indicates which issue it solved

sdk/python/kfp/_client.py

Bobgy · 2020-07-06T08:54:40Z

/lgtm
This is perfect!

/assign @Ark-kun @numerology
Do you want to further review?

I think this is good to go.

numerology

Thanks! @NikeNano

sdk/python/kfp/_client.py

numerology · 2020-07-06T22:51:56Z

sdk/python/kfp/_client.py

+    # Operators on scalar values. Only applies to one of |int_value|,
+    # |long_value|, |string_value| or |timestamp_value|.
+    "EQUALS" : 1,
+    "NOT_EQUALS" : 2,


Just for my education, is the predicates other than 'EQUALS' being used now in the client?

No, they are not I kept them on order to have a reference for the alternative since we don't have a proper helper file. Maybe we should remove it. Initially I only. set it to 1, but @Bobgy suggested that we added a mapping in order for readers of the code to understand what it meant.

sdk/python/kfp/_client.py

sdk/python/kfp/cli/pipeline.py

Co-authored-by: Jiaxiao Zheng <jxzheng@google.com>

Bobgy · 2020-07-08T05:57:18Z

/lgtm
/approve

Thanks!

k8s-ci-robot · 2020-07-08T05:57:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bobgy

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:

~~sdk/OWNERS~~ [Bobgy]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Bobgy · 2020-07-08T10:35:34Z

@googlebot rescan

…ubeflow#3901 (kubeflow#4087) * Update _client.py * Allow for passing name instead of only pipeline_id * fixed old rebase issue * protobuf fix * raise error and remove f-string * moved file * updated python proto packages versions * Update sdk/python/kfp/_client.py * restructured and added to build * changes structure of import * Updated the files * further updates of client and filter.proto * alternative * clean up * clean up * remove helperfiles * futher clean up * clean up or eaither name or id * update doc strings * remove page_size * Update sdk/python/kfp/_client.py Co-authored-by: Jiaxiao Zheng <jxzheng@google.com> * Update sdk/python/kfp/cli/pipeline.py Co-authored-by: Jiaxiao Zheng <jxzheng@google.com> * updated to classical string formattin Co-authored-by: Jiaxiao Zheng <jxzheng@google.com>

Niklas Hansson added 3 commits June 26, 2020 09:33

Update _client.py

8eae9c1

Allow for passing name instead of only pipeline_id

33d3ac4

fixed old rebase issue

91726cf

k8s-ci-robot requested review from Ark-kun and gaoning777 June 26, 2020 08:53

k8s-ci-robot added the size/M label Jun 26, 2020

Bobgy reviewed Jun 26, 2020

View reviewed changes

sdk/python/kfp/_client.py Show resolved Hide resolved

Bobgy reviewed Jun 26, 2020

View reviewed changes

sdk/python/kfp/_client.py Outdated Show resolved Hide resolved

protobuf fix

13df138

k8s-ci-robot added size/L and removed size/M labels Jun 26, 2020

Bobgy mentioned this pull request Jun 28, 2020

Ability to List Resources by Name Instead of Id #4069

Closed

Ark-kun reviewed Jun 30, 2020

View reviewed changes

sdk/python/kfp/cli/pipeline.py Outdated Show resolved Hide resolved

Ark-kun reviewed Jun 30, 2020

View reviewed changes

sdk/python/kfp/cli/pipeline.py Outdated Show resolved Hide resolved

Ark-kun reviewed Jun 30, 2020

View reviewed changes

sdk/python/kfp/cli/pipeline.py Outdated Show resolved Hide resolved

Ark-kun reviewed Jun 30, 2020

View reviewed changes

sdk/python/kfp/cli/pipeline.py Outdated Show resolved Hide resolved

raise error and remove f-string

cb03f8b

googlebot added the cla: yes label Jun 30, 2020

NikeNano added 2 commits July 1, 2020 16:15

moved file

43603ec

updated python proto packages versions

3662432

NikeNano commented Jul 1, 2020

View reviewed changes

sdk/python/kfp/_client.py Outdated Show resolved Hide resolved

Update sdk/python/kfp/_client.py

b5815c3

Bobgy reviewed Jul 3, 2020

View reviewed changes

Bobgy changed the title ~~feat(Cli). Update to allow upload_version to be done using only the name.~~ feat(sdk): CLI - Update to allow upload_version to be done using only the name. Jul 3, 2020

remove helperfiles

ee829f4

k8s-ci-robot added size/M and removed size/XL labels Jul 6, 2020

futher clean up

30402b5

Bobgy reviewed Jul 6, 2020

View reviewed changes

sdk/python/kfp/cli/pipeline.py Show resolved Hide resolved

sdk/python/kfp/_client.py Outdated Show resolved Hide resolved

clean up or eaither name or id

5492df7

Bobgy reviewed Jul 6, 2020

View reviewed changes

Bobgy changed the title ~~feat(sdk): CLI - Update to allow upload_version to be done using only the name.~~ feat(sdk): CLI - Allow upload_version by the pipeline's name. Fixes #3901 Jul 6, 2020

update doc strings

99d32e6

Bobgy reviewed Jul 6, 2020

View reviewed changes

sdk/python/kfp/_client.py Outdated Show resolved Hide resolved

remove page_size

d543400

k8s-ci-robot assigned Ark-kun, numerology and Bobgy Jul 6, 2020

k8s-ci-robot added the lgtm label Jul 6, 2020

numerology reviewed Jul 6, 2020

View reviewed changes

Update sdk/python/kfp/_client.py

778f3ed

Co-authored-by: Jiaxiao Zheng <jxzheng@google.com>

k8s-ci-robot removed the lgtm label Jul 7, 2020

Niklas Hansson and others added 2 commits July 7, 2020 14:23

Update sdk/python/kfp/cli/pipeline.py

0ab53b0

Co-authored-by: Jiaxiao Zheng <jxzheng@google.com>

updated to classical string formattin

330eacb

k8s-ci-robot added the lgtm label Jul 8, 2020

k8s-ci-robot added the approved label Jul 8, 2020

k8s-ci-robot merged commit aae1eba into kubeflow:master Jul 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sdk): CLI - Allow upload_version by the pipeline's name. Fixes #3901 #4087

feat(sdk): CLI - Allow upload_version by the pipeline's name. Fixes #3901 #4087

NikeNano commented Jun 26, 2020

kubeflow-bot commented Jun 26, 2020

NikeNano commented Jun 26, 2020

NikeNano commented Jun 30, 2020

NikeNano commented Jul 1, 2020

NikeNano commented Jul 1, 2020

Bobgy left a comment

Bobgy commented Jul 3, 2020

Bobgy left a comment

Bobgy Jul 6, 2020

NikeNano Jul 6, 2020

Bobgy commented Jul 6, 2020

Bobgy commented Jul 6, 2020

Bobgy commented Jul 6, 2020

numerology left a comment

numerology Jul 6, 2020

NikeNano Jul 7, 2020

Bobgy commented Jul 8, 2020

k8s-ci-robot commented Jul 8, 2020

Bobgy commented Jul 8, 2020

feat(sdk): CLI - Allow upload_version by the pipeline's name. Fixes #3901 #4087

feat(sdk): CLI - Allow upload_version by the pipeline's name. Fixes #3901 #4087

Conversation

NikeNano commented Jun 26, 2020

kubeflow-bot commented Jun 26, 2020

NikeNano commented Jun 26, 2020

NikeNano commented Jun 30, 2020

NikeNano commented Jul 1, 2020

NikeNano commented Jul 1, 2020

Bobgy left a comment

Choose a reason for hiding this comment

Bobgy commented Jul 3, 2020

Bobgy left a comment

Choose a reason for hiding this comment

Bobgy Jul 6, 2020

Choose a reason for hiding this comment

NikeNano Jul 6, 2020

Choose a reason for hiding this comment

Bobgy commented Jul 6, 2020

Bobgy commented Jul 6, 2020

Bobgy commented Jul 6, 2020

numerology left a comment

Choose a reason for hiding this comment

numerology Jul 6, 2020

Choose a reason for hiding this comment

NikeNano Jul 7, 2020

Choose a reason for hiding this comment

Bobgy commented Jul 8, 2020

k8s-ci-robot commented Jul 8, 2020

Bobgy commented Jul 8, 2020