fix(frontend): fixes filter pipeline text box shows error when typing anything in it. Fixes #10241

[ElayAharoni]

fixes filter pipeline text box shows error when typing anything in it.

fix for issue #10241.

just did small change to use the V2 code instead the V1

[google-oss-prow]

Hi @ElayAharoni. 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.

[hbelmiro]

/ok-to-test

[andreyvelich]

/ok-to-test

Add /rerun-all comment to run the actions

[ElayAharoni]

/rerun-all

[ElayAharoni]

please if someone can review the PR when possible.
@zijianjoy , @HumairAK

[boarder7395]

@ElayAharoni Take a look at my comment on the original issue #10241 (comment)

While this does fix the create run page, it will break the clone from run page.

Steps to reproduce with your change.

• Open Runs page.
• Click on an existing pipeline Run
• Click "Clone Run"
• Click "Choose" on the pipeline (or experiment).
• Enter search text.

[HumairAK]

@boarder7395 I was unable to reproduce your issue with clone run, can you describe what your environment a bit more? (let's discuss the pr implementation and any issues associated with it, in this thread)

I tried this on standalone and it seemed to work fine:
Before:

After:

With clone run:

[HumairAK]

While this does work for listing v2 pipelines, I'm guessing it breaks it for v1 pipelines. I'm trying to wrap my head around why this isCalledByV1 exists, because you can either listPipelines based off v1 api or v2 api, but not both, yet we can have v1/v2 pipelines uploaded together in KFP, which makes me think that we actually don't properly support listing/filtering for both kfp v1 and v2 pipelines together

@zijianjoy does that sound right to you?

either way I would rather we have this work with v2 rather than v1, and discuss a better design approach on how to handle this case for when we have both pipelines uploaded and need to list/filter simultaneously

@zijianjoy wdyt?

[zijianjoy]

isCalledByV1 was introduced to differentiate API request payload schema between V1 API and V2 API.

pipelines/frontend/src/components/CustomTable.tsx

Lines 229 to 246 in 1c7954d

	this.state = {
	currentPage: 0,
	filterString: this.props.initialFilterString || '',
	filterStringEncoded:
	this.props.initialFilterString && this.props.isCalledByV1
	? this._createAndEncodeFilterV1(this.props.initialFilterString)
	: this.props.initialFilterString
	? this._createAndEncodeFilterV2(this.props.initialFilterString)
	: '',
	isBusy: false,
	maxPageIndex: Number.MAX_SAFE_INTEGER,
	pageSize: 10,
	sortBy:
	props.initialSortColumn || (props.columns.length ? props.columns[0].sortKey || '' : ''),
	sortOrder: props.initialSortOrder || 'desc',
	tokenList: [''],
	};
	}

. Because we have a flag to control whether to show v2 UI by default during the initial migration from v1 to v2.

Ideally v2 should support filer for V1 names filtering as well. So it makes sense to me for switching to V2 filter mechanism.

[boarder7395]

@HumairAK You're right I was using clone from run with a pipeline created in V1.

When I use a V2 pipeline the functionality works as expected. I can understand merging this since it does solve the "Create Run" issue, which we've actually implemented on our fork as well, since that is a bigger issue than it breaking the "Clone from V1 Pipeline" page.

Should I open a separate issue on the broken "Clone from v1 pipeline" search field?

Also what tool are you using for browser recording, that would be super helpful for future issues.

[HumairAK]

@boarder7395

Should I open a separate issue on the broken "Clone from v1 pipeline" search field?

If we merge this I think we would need a follow up issue for resolving search fields for when v1 pipelines are present. I've prompted @zijianjoy for suggestions here, so let's wait for a response.

Also what tool are you using for browser recording, that would be super helpful for future issues.

I'm on linux, and use a tool called peek, it's a super basic tool, but it certainly fits my needs 😄

[zijianjoy]

/lgtm
/approve

Sounds good to me for fixing V2 scenario first. Please feel free to create a separate issue if mixing with V1 pipeline causing problem.

[google-oss-prow]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: zijianjoy

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:

• frontend/OWNERS [zijianjoy]

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