Skip to content
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

Use pagination helper to list pipelines in cli #4478

Merged
merged 6 commits into from
Dec 1, 2024
Merged

Conversation

xoxys
Copy link
Member

@xoxys xoxys commented Nov 29, 2024

This PR fixes the woodecker-cli pipeline ls command. Before this change it was no possible to list more than 50 pipelines as this is the hardcoded limit of the API for pipelines per page.

@woodpecker-bot
Copy link
Contributor

woodpecker-bot commented Nov 29, 2024

Deployment of preview was torn down

@xoxys xoxys requested a review from a team November 29, 2024 20:28
@xoxys xoxys added enhancement improve existing features cli labels Nov 29, 2024
@qwerty287
Copy link
Contributor

This doesn't work with e.g. branch and event filter. The limit is not applied to these. If you want 10 pipelines for main branch, it will just load 10 pipelines and then remove the other ones. So it will be less than 10. Didn't we just introduce server-side filtering?

@xoxys
Copy link
Member Author

xoxys commented Nov 30, 2024

Yes I know but this was already the case before thos change.

Yes there is now server side filtering but not for all three options and I would now look into the open autogen client PR instead of copying everything manually to woodpecker-go again.

So can we merge this PR as is?

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

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

Well, previously it was always loading the first page without any limit (afaik the default is 25). If you had a lower limit, it was working, now it will be always less if something is filtered out. But if we fix it soon it's fine for me.

@xoxys
Copy link
Member Author

xoxys commented Nov 30, 2024

You are right. Then lets keep it open for now and I will look into to clientgen first.

@6543
Copy link
Member

6543 commented Nov 30, 2024

... not for all three options ...

the woodpecker-go currently does not have support for the new filters yes ... I'm also looking forward to generate it

@xoxys
Copy link
Member Author

xoxys commented Nov 30, 2024

Depends-on: #4494

@xoxys xoxys force-pushed the cli-paginate-pipelines branch from a6c8769 to c3fbe67 Compare November 30, 2024 23:38
@xoxys xoxys force-pushed the cli-paginate-pipelines branch from c3fbe67 to 3aa6934 Compare November 30, 2024 23:40
@pat-s pat-s merged commit 3c31284 into main Dec 1, 2024
6 of 7 checks passed
@pat-s pat-s deleted the cli-paginate-pipelines branch December 1, 2024 19:39
@woodpecker-bot woodpecker-bot mentioned this pull request Dec 1, 2024
1 task
@woodpecker-bot woodpecker-bot mentioned this pull request Dec 14, 2024
1 task
pat-s pushed a commit that referenced this pull request Dec 18, 2024
Co-authored-by: 6543 <6543@obermui.de>
pat-s pushed a commit that referenced this pull request Dec 18, 2024
Co-authored-by: 6543 <6543@obermui.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli enhancement improve existing features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants