feat: Search by name for WorkflowTemplates in UI #11684

sunyeongchoi · 2023-08-27T07:24:33Z

Fixes #11004

Motivation

It was implemented because the function to filter the result of the workflow template by name was considered necessary.

If the corresponding PR is merged, the same will be applied to other resource pages.

Modifications

It was implemented by adding a namePattern field to search based on name contains.
Implementation of cursor pagination logic based on resourceVersion to newly paginate the results of filtering on the kubernetes list results.

Verification

Pass workflow_template_server_test.go
UI workflow template first page normal
Normal when the workflow template page limit is 5, 10, or 50
Test the above with name pattern filtering search
After searching without a name pattern, search again by adding a name pattern
After searching including the name pattern, delete the name pattern and search again

…plate Signed-off-by: shmruin <meme_hm@naver.com>

Signed-off-by: sunyeongchoi <sn0716@naver.com>

Signed-off-by: leeeuijoo <euijoo3233@gmail.com>

Signed-off-by: shmruin <meme_hm@naver.com>

Signed-off-by: sunyeongchoi <sn0716@naver.com>

unused namePattern props delete, change inputFilter name to camelcase Signed-off-by: sunyeongchoi <sn0716@naver.com>

Signed-off-by: sunyeongchoi <sn0716@naver.com>

terrytangyuan · 2023-08-28T15:14:19Z

server/workflowtemplate/workflow_template_server.go

+	// Sort by resourceVersion desc
+	sort.Slice(items, func(i, j int) bool {
+		itemIRV, _ := strconv.Atoi(items[i].ResourceVersion)
+		itemJRV, _ := strconv.Atoi(items[j].ResourceVersion)
+		return itemIRV > itemJRV
+	})
+
+	// Do resourceVersion filtering if continue exist
+	if resourceVersion != "" {
+		newItems := []v1alpha1.WorkflowTemplate{}
+		for _, item := range items {
+			targetRV, _ := strconv.Atoi(item.ResourceVersion)
+			receivedRV, _ := strconv.Atoi(resourceVersion)
+			if targetRV < receivedRV {
+				newItems = append(newItems, item)
+			}
+			items = newItems
+		}
+	}
+
+	// Indexing list by limit count
+	if limit != 0 {
+		endIndex := int(limit)
+		if endIndex > len(items) || limit == 0 {
+			endIndex = len(items)
+		}
+		wfList.Items = items[0:endIndex]
+	} else {
+		wfList.Items = items
+	}
+
+	// Calculate new offset for next batch
+	if limit != 0 && len(wfList.Items) == int(limit) {
+		wfList.ListMeta.Continue = wfList.Items[len(wfList.Items)-1].ResourceVersion
+	}


Can you add some explanation of this implementation in the comments?

@terrytangyuan
Of course!
We have implemented namePattern filtering.
Since Kubernetes doesn't provide filtering like namePattern, we implemented logic to retrieve results from Kubernetes, then separately filter the results.

In the previous logic, we were utilizing the "continue" option for Kubernetes' built-in pagination.
However, when customizing Kubernetes' results, features like namePattern disrupt the proper usage of "continue" pagination.

Hence, we implemented our own pagination by utilizing Kubernetes' resourceVersion for cursor pagination.

Sort in descending order based on resourceVersion.

Filter only smaller values compared to the resourceVersion held from the previous page.

Index the filtered results based on the limit.

Store the resourceVersion for use in the next page.

The pagination logic is implemented in the sequence described above. Thank you.

Also I added a comment about the logic and committed it. e939bb8

Signed-off-by: sunyeongchoi <sn0716@naver.com>

terrytangyuan · 2023-09-11T00:22:13Z

server/workflowtemplate/workflow_template_server.go

+	// Search whole with Limit 0.
+	// Reset the Continue "" to prevent Kubernetes native pagination.
+	// Kubernetes api will search for all results without limit and pagination.
+	k8sOptions.Continue = ""
+	k8sOptions.Limit = 0


Would it be possible to stick with the native pagination when search functionality is not used?

I think maybe it is possible. I will try to implement it like that.

I uploaded commit about sticking with native pagination when search functionality is not used. 392222b

If this PR is reflected, we plan to implement the same namePattern function on other pages as well. I will consider having cursorPaginatonByResourceVersion common in one place and one function.

…ement-filter-by-name Signed-off-by: sunyeongchoi <sn0716@naver.com>

…ot used Signed-off-by: sunyeongchoi <sn0716@naver.com>

terrytangyuan

Thanks! Great work 👍

agilgur5 · 2024-01-29T05:23:56Z

server/workflowtemplate/workflow_template_server.go

+	if req.NamePattern != "" {
+		// Search whole with Limit 0.
+		// Reset the Continue "" to prevent Kubernetes native pagination.
+		// Kubernetes api will search for all results without limit and pagination.
+		k8sOptions.Continue = ""
+		k8sOptions.Limit = 0


correct me if I'm wrong, but won't this have the same problem as #11761 / i.e. #12025 when there are enough WorkflowTemplates? It looks like the exact problem was mentioned in #11004 (comment)

The workaround here may be to only retrieve the limit at a time from the k8s API. If there aren't enough after the filter, then retrieve more.

That might make the request take longer (potentially significantly, but maybe less than retrieving all at once, which also takes time, and, crucially, memory), but would make sure we never retrieve too many (i.e. it throttles it, effectively). There are some other side-effects to that too (the k8s API rate limit, for instance).

The other way may be to do this fully client-side or partially client-side.
Fully would be the UI (and CLI? a flag wasn't implemented here though) fetches and does a filter, and then if it doesn't have enough, requests more.
Partially would be the API does a filter but can underfetch (it only filters on the limit), in which case the UI has to request more by moving up the cursor.

I'm also thinking we may want to explicitly point out in the UI which search fields are not k8s native and can therefore be non-performant when used; basically a "hey we implemented this, but you might want to be careful using this".

and at that point, potentially let admins disable them with flags too, if they don't want users to overload the Argo Server or k8s API accidentally.

For future reference, the current pattern to mitigate this would be to follow the SQLite pattern here a la #12736, see also #13633 (comment) et al

agilgur5 · 2024-06-22T20:43:16Z

pkg/apiclient/workflowtemplate/workflow-template.proto

-  k8s.io.apimachinery.pkg.apis.meta.v1.ListOptions listOptions = 2;
+  string namePattern = 2;
+  k8s.io.apimachinery.pkg.apis.meta.v1.ListOptions listOptions = 3;


changing the tag numbers is apparently a breaking change in protobufs, see #13160 (comment) for more details.

agilgur5 · 2024-06-22T20:44:18Z

...rc/app/workflow-templates/components/workflow-template-filters/workflow-template-filters.tsx

+                    />
+                </div>
+                <div className='columns small-2 xlarge-12'>
+                    <p className='wf-filters-container__title'>Name Pattern</p>


"Pattern" is a bit ambiguous as it can mean several things. "Contains" is a more explicit naming. See #13160 (comment) for more details

shmruin and others added 8 commits August 27, 2023 14:12

feat: Change client-side api to support searching by name workflowTem…

017d8a7

…plate Signed-off-by: shmruin <meme_hm@naver.com>

feat: Add filtering by namePattern parameter to ListWorkflowTemplates

162be8b

Signed-off-by: sunyeongchoi <sn0716@naver.com>

feat: Add workflow-template Filter components

dbeeab5

Signed-off-by: leeeuijoo <euijoo3233@gmail.com>

feat: Add new pagination logic

890d882

Signed-off-by: shmruin <meme_hm@naver.com>

feat: pagintation by resourceVersion

16c70d2

Signed-off-by: sunyeongchoi <sn0716@naver.com>

fix: namePattern caching logic update in frontend

0f2f21d

unused namePattern props delete, change inputFilter name to camelcase Signed-off-by: sunyeongchoi <sn0716@naver.com>

fix: resourceVersion pagination code refactoring

afe6159

Signed-off-by: sunyeongchoi <sn0716@naver.com>

fix: reset pagination offset when input filter onChange

65448b2

Signed-off-by: sunyeongchoi <sn0716@naver.com>

sunyeongchoi requested review from jessesuen, alexec, terrytangyuan and sarabala1979 as code owners August 27, 2023 07:24

sunyeongchoi changed the title ~~Cherrypick/enhancement filter by name~~ feat: Search by name for WorkflowTemplates in UI Aug 27, 2023

sunyeongchoi force-pushed the cherrypick/enhancement-filter-by-name branch 3 times, most recently from 4f11b7e to ddd63fd Compare August 28, 2023 07:00

fix: ui lint

f387879

Signed-off-by: sunyeongchoi <sn0716@naver.com>

sunyeongchoi force-pushed the cherrypick/enhancement-filter-by-name branch from ddd63fd to f387879 Compare August 28, 2023 08:50

sunyeongchoi closed this Aug 28, 2023

sunyeongchoi reopened this Aug 28, 2023

terrytangyuan reviewed Aug 28, 2023

View reviewed changes

feat: add explanation of namePattern filtering list logic

e939bb8

Signed-off-by: sunyeongchoi <sn0716@naver.com>

terrytangyuan mentioned this pull request Aug 30, 2023

3.5 Pagination may not work correctly for archived workflows #11715

Closed

3 tasks

terrytangyuan reviewed Sep 11, 2023

View reviewed changes

agilgur5 added area/api Argo Server API area/ui labels Sep 11, 2023

sunyeongchoi added 2 commits September 17, 2023 21:40

Merge remote-tracking branch 'upstream/master' into cherrypick/enhanc…

9652ee2

…ement-filter-by-name Signed-off-by: sunyeongchoi <sn0716@naver.com>

feat: stick with the native pagination when search functionality is n…

392222b

…ot used Signed-off-by: sunyeongchoi <sn0716@naver.com>

terrytangyuan approved these changes Sep 17, 2023

View reviewed changes

terrytangyuan merged commit 246d4f4 into argoproj:master Sep 17, 2023
22 checks passed

sunyeongchoi mentioned this pull request Oct 29, 2023

REQUEST: New membership for @sunyeongchoi argoproj/argoproj#250

Closed

5 tasks

agilgur5 added the area/workflow-templates label Jan 29, 2024

agilgur5 reviewed Jan 29, 2024

View reviewed changes

Adrien-D mentioned this pull request Jun 11, 2024

feat: Add name filters for workflow list (exact/prefix/pattern) #13160

Merged

agilgur5 reviewed Jun 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Search by name for WorkflowTemplates in UI #11684

feat: Search by name for WorkflowTemplates in UI #11684

sunyeongchoi commented Aug 27, 2023

terrytangyuan Aug 28, 2023

sunyeongchoi Aug 29, 2023

terrytangyuan Sep 11, 2023

sunyeongchoi Sep 13, 2023

sunyeongchoi Sep 17, 2023

terrytangyuan left a comment

agilgur5 Jan 29, 2024

agilgur5 Jan 29, 2024 •

edited

Loading

agilgur5 Jan 29, 2024

agilgur5 Nov 10, 2024

agilgur5 Jun 22, 2024

agilgur5 Jun 22, 2024

feat: Search by name for WorkflowTemplates in UI #11684

feat: Search by name for WorkflowTemplates in UI #11684

Conversation

sunyeongchoi commented Aug 27, 2023

Motivation

Modifications

Verification

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

terrytangyuan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agilgur5 Jan 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

agilgur5 Jan 29, 2024 •

edited

Loading