feat(sdk): Add filters to python client. #6748

jmcarp · 2021-10-15T02:32:12Z

Description of your changes:

The backend and generated python code accept filters for resource list requests, but we don't expose those in the client. This patch exposes filters as appropriate as json-encoded strings. Note: we might want to accept filters as lists of dicts, or we could generate python classes from filters.proto and pass in Filter objects instead. Any preference on this?

Checklist:

The title for your pull request (PR) should follow our title convention. Learn more about the pull request title convention used in this repository.

google-oss-robot · 2021-10-15T02:32:24Z

Hi @jmcarp. Thanks for your PR.

I'm waiting for a kubeflow member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

chensun

The backend and generated python code accept filters for resource list requests, but we don't expose those in the client.

Just curious, when was the filter option added? Is there a minimum version of backend/API that we need to be aware of for this change?

chensun · 2021-10-19T16:42:32Z

sdk/python/kfp/_client.py

@@ -506,7 +506,8 @@ def list_experiments(self,
                         page_token='',
                         page_size=10,
                         sort_by='',
-                         namespace=None):
+                         namespace=None,
+                         filter=None):


Can you please document this argument below in the function docstring?

The link there is now broken.

https://github.com/kubeflow/pipelines/blob/master/backend/api/filter.proto

chensun · 2021-10-19T16:42:50Z

sdk/python/kfp/_client.py

@@ -1061,7 +1066,8 @@ def list_runs(self,
                  page_size=10,
                  sort_by='',
                  experiment_id=None,
-                  namespace=None):
+                  namespace=None,
+                  filter=None):


ditto: docstring

chensun · 2021-10-19T16:43:02Z

sdk/python/kfp/_client.py

        return response

    def list_recurring_runs(self,
                            page_token='',
                            page_size=10,
                            sort_by='',
-                            experiment_id=None):
+                            experiment_id=None,
+                            filter=None):


chensun · 2021-10-19T17:25:06Z

The backend and generated python code accept filters for resource list requests, but we don't expose those in the client.

Just curious, when was the filter option added? Is there a minimum version of backend/API that we need to be aware of for this change?

I took a look, seems to be there for a long time, no need to worry about the minimum version.

Can you please add an entry in the release note under "Current version" ?

https://github.com/kubeflow/pipelines/blob/master/sdk/RELEASE.md

jmcarp · 2021-10-26T23:28:03Z

Updated @chensun.

chensun

/lgtm

Thanks!

chensun · 2021-10-27T05:11:44Z

sdk/RELEASE.md

@@ -17,6 +17,7 @@

 * Fix placeholder mapping error in v2. [\#6794](https://github.com/kubeflow/pipelines/pull/6794)
 * Depends on `kfp-pipeline-spec>=0.1.13,<0.2.0` [\#6803](https://github.com/kubeflow/pipelines/pull/6803)
+* Add optional `filter` argument to list methods of KFP client.


nit: Add the PR link at the end just like the rest? See if you want to edit this yourself, or I can help as well.

@chensun

h/t @chensun

chensun · 2021-10-28T06:13:28Z

/lgtm
/approve

Thanks!

google-oss-robot · 2021-10-28T06:13:35Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chensun

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~~ [chensun]

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

chensun · 2021-10-28T06:57:20Z

/lgtm

google-oss-robot · 2021-10-28T07:38:55Z

@jmcarp: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
kubeflow-pipelines-samples-v2	`76fcebf`	link	true	`/test kubeflow-pipelines-samples-v2`

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

chensun · 2021-10-28T08:15:45Z

Adding another data point to #6815

https://4e18c21c9d33d20f-dot-datalab-vm-staging.googleusercontent.com/#/runs/details/d1bb5bb3-f157-469b-9058-93d234e4d30f

One of the print_op in a loop failed with error:

failed to try resolving symlinks in path "/var/log/pods/kubeflow_my-pipeline-bgdnw-2290115955_fb33b214-5e9c-4287-83d9-019e0e140801/main/0.log": lstat /var/log/pods/kubeflow_my-pipeline-bgdnw-2290115955_fb33b214-5e9c-4287-83d9-019e0e140801/main/0.log: no such file or directory

All the rest print_ops succeeded.

Going to ignore the test failure and merge as-is.

JakeSummers · 2023-02-01T16:44:24Z

This feature would be much easier to use if there were Python types available for the filter. See #8778

google-cla bot added the cla: yes label Oct 15, 2021

google-oss-robot added size/M needs-ok-to-test labels Oct 15, 2021

google-oss-robot requested review from chensun and ji-yaqi October 15, 2021 02:32

chensun reviewed Oct 19, 2021

View reviewed changes

Add filters to python client.

29e8ed9

jmcarp force-pushed the jmcarp/api-filter branch from 4d8d925 to 8830060 Compare October 26, 2021 23:27

chensun reviewed Oct 27, 2021

View reviewed changes

google-oss-robot assigned chensun Oct 27, 2021

google-oss-robot added the lgtm label Oct 27, 2021

Add filter docstrings.

2fdfb61

h/t @chensun

jmcarp force-pushed the jmcarp/api-filter branch from 8830060 to 2fdfb61 Compare October 28, 2021 00:04

google-oss-robot removed the lgtm label Oct 28, 2021

google-oss-robot added the lgtm label Oct 28, 2021

google-oss-robot added the approved label Oct 28, 2021

chensun removed the needs-ok-to-test label Oct 28, 2021

Merge branch 'master' into jmcarp/api-filter

76fcebf

google-oss-robot removed the lgtm label Oct 28, 2021

google-oss-robot added the lgtm label Oct 28, 2021

chensun merged commit 37d3234 into kubeflow:master Oct 28, 2021

alexlatchford mentioned this pull request Jan 5, 2022

[Part 1 of 2] AIP-5426: Revert prior KFP CLI fixes zillow/pipelines#14

Merged

alexlatchford mentioned this pull request Feb 2, 2022

fix(sdk): Add in filter to list_pipeline_versions SDK method #7223

Merged

JakeSummers mentioned this pull request Feb 1, 2023

[feature] Add dataclass for filter #8778

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(sdk): Add filters to python client. #6748

feat(sdk): Add filters to python client. #6748

jmcarp commented Oct 15, 2021

google-oss-robot commented Oct 15, 2021

chensun left a comment

chensun Oct 19, 2021

jmcarp Oct 26, 2021

JakeSummers Feb 1, 2023

chensun Oct 19, 2021

chensun Oct 19, 2021

chensun commented Oct 19, 2021

jmcarp commented Oct 26, 2021

chensun left a comment

chensun Oct 27, 2021

jmcarp Oct 28, 2021

chensun commented Oct 28, 2021

google-oss-robot commented Oct 28, 2021

chensun commented Oct 28, 2021

google-oss-robot commented Oct 28, 2021

chensun commented Oct 28, 2021

JakeSummers commented Feb 1, 2023

feat(sdk): Add filters to python client. #6748

feat(sdk): Add filters to python client. #6748

Conversation

jmcarp commented Oct 15, 2021

google-oss-robot commented Oct 15, 2021

chensun left a comment

Choose a reason for hiding this comment

chensun Oct 19, 2021

Choose a reason for hiding this comment

jmcarp Oct 26, 2021

Choose a reason for hiding this comment

JakeSummers Feb 1, 2023

Choose a reason for hiding this comment

chensun Oct 19, 2021

Choose a reason for hiding this comment

chensun Oct 19, 2021

Choose a reason for hiding this comment

chensun commented Oct 19, 2021

jmcarp commented Oct 26, 2021

chensun left a comment

Choose a reason for hiding this comment

chensun Oct 27, 2021

Choose a reason for hiding this comment

jmcarp Oct 28, 2021

Choose a reason for hiding this comment

chensun commented Oct 28, 2021

google-oss-robot commented Oct 28, 2021

chensun commented Oct 28, 2021

google-oss-robot commented Oct 28, 2021

chensun commented Oct 28, 2021

JakeSummers commented Feb 1, 2023