8000 fix(cli): generate UserWarning if `list` does not return all entries by JohnVillalovos · Pull Request #2901 · python-gitlab/python-gitlab · GitHub
[go: up one dir, main page]
Skip to content

fix(cli): generate UserWarning if list does not return all entries #2901

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jul 4, 2024
Merged

fix(cli): generate UserWarning if list does not return all entries #2901

merged 3 commits into from
Jul 4, 2024

Conversation

JohnVillalovos
Copy link
Member

Previously in the CLI, calls to list() would have get_all=False by
default. Therefore hiding the fact that not all items are being
returned if there were more than 20 items.

Added --no-get-all option to list actions. Along with the already
existing --get-all.

Closes: #2900

@JohnVillalovos JohnVillalovos requested a review from nejch June 22, 2024 00:49
@JohnVillalovos JohnVillalovos force-pushed the jlvillal/cli_warn branch 3 times, most recently from 05c0c70 to 09b9205 Compare July 1, 2024 16:05
nejch
nejch reviewed Jul 3, 2024
View reviewed changes
Copy link
Member
@nejch nejch left a comment

Choose a reason for hiding this comment

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

THanks a lot @JohnVillalovos, I've been thinking about this one for a bit because we reintroduce more CLI logic into the API client, and I wonder if there's a more generic way. Just a tiny idea below 🙇

@JohnVillalovos
chore: use correct type-hint for die()
b02411a
@JohnVillalovos
chore: add show_caller argument to utils.warn()
25cdeee 
This allows us to not add the caller's location to the UserWarning
message.
@JohnVillalovos
fix(cli): generate UserWarning if list does not return all entries
e1e0060 
Previously in the CLI, calls to `list()` would have `get_all=False` by
default. Therefore hiding the fact that not all items are being
returned if there were more than 20 items.

Added `--no-get-all` option to `list` actions. Along with the already
existing `--get-all`.

Closes: #2900
@JohnVillalovos
Copy link
Member Author

THanks a lot @JohnVillalovos, I've been thinking about this one for a bit because we reintroduce more CLI logic into the API client, and I wonder if there's a more generic way. Just a tiny idea below 🙇

@nejch thanks for the review. I have updated it to pass in a WarnMessageData object. So I think it will do what you want.

@JohnVillalovos JohnVillalovos requested a review from nejch July 3, 2024 23:51
@nejch nejch merged commit e5a4379 into main Jul 4, 2024
16 checks passed
@nejch nejch deleted the jlvillal/cli_warn branch July 4, 2024 08:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nejch nejch nejch approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CLI: For list actions the CLI defaults to get_all=False
2 participants
@JohnVillalovos @nejch
0