Skip to content
New issue

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

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

Already on GitHub? # to your account

Specify pagesize for workflow list command #748

Merged
merged 4 commits into from
Feb 20, 2025

Conversation

yiminc
Copy link
Member

@yiminc yiminc commented Feb 7, 2025

What was changed

Add optional pagesize parameter to workflow list command

Why?

Be able to specify page size is useful when default page is too large for cases where it leads to timeout due to large amount of data.

Checklist

  1. Closes

  2. How was this tested:

Updated unit test to cover it.

  1. Any docs updates needed?

@yiminc yiminc force-pushed the vis_list_pagesize branch from bcd8ab6 to 9504ce3 Compare February 7, 2025 02:48
@@ -2886,6 +2886,9 @@ commands:
- name: limit
type: int
description: Maximum number of Workflow Executions to display.
- name: pagesize
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be consistent with how we name other flags, this should be:

Suggested change
- name: pagesize
- name: page-size

@@ -2886,6 +2886,9 @@ commands:
- name: limit
type: int
description: Maximum number of Workflow Executions to display.
- name: pagesize
type: int
description: Maximum number of Workflow Executions to return from server per request. Used for pagination.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional; rewords in terms of behavior, not implementation.

Suggested change
description: Maximum number of Workflow Executions to return from server per request. Used for pagination.
description: Maximum number of Workflow Executions to fetch at a time from the server.

Copy link
Collaborator

@josh-berry josh-berry left a comment

Choose a reason for hiding this comment

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

LGTM (assuming we decide to stay with the naming etc. per offline discussion)

@@ -2886,6 +2886,9 @@ commands:
- name: limit
type: int
description: Maximum number of Workflow Executions to display.
- name: page-size
Copy link
Member

@cretz cretz Feb 10, 2025

Choose a reason for hiding this comment

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

Suggested change
- name: page-size
- name: force-override-page-size

Mentioned internally, but this less-wieldy form may make it a bit clearer users usually don't ever need to set this. No strong opinion though.

@yiminc yiminc merged commit 1cb1264 into temporalio:main Feb 20, 2025
6 of 7 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants