-
Notifications
You must be signed in to change notification settings - Fork 22
Pagination support for listing workflows through a new Client#list_workflow_page method #285
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pagination support for listing workflows through a new Client#list_workflow_page method #285
Conversation
|
|
cretz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Mentioned in comment, let's think on design of this just a bit. I opened an issue to track the work.
cretz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Only a few hopefully-minor things.
|
It looks like there may be CI failures for two things not related to this issue (generated protos have a bit of a new format it seems and Rubocop just released a new check). This is kinda our fault for not using a lock file and always running CI, intentionally, against latest versions. I will fix in #291 before I merge it and then will merge main here (and #286). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for sticking with this. Only minor change is to remove < Data. I am working on getting CI fixed in another PR.
cretz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thought this was a straightforward enough change, but I am happy to also open an issue if more discussions are needed.
What was changed
Added support for getting a list of workflows one page at a time in a new method
Client#list_workflow_page. Example:Why?
Fetching all workflow executions matching a query can be extremely expensive if there are a lot of matches. The risk of hitting rate limit or other RPC errors is high, and a lot of the time getting all the results is not necessary.
For example, in a user interface showing a list of workflows one page at a time, we want to only get the list of workflows for the currently shown page.