CENG-355: Add --show-all flag to display all results from repo/package list
#172
Conversation
…71' of github.com:cloudsmith-io/cloudsmith-cli into ceng-355-httpsgithubcomcloudsmith-iocloudsmith-clipull171
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| @@ -110,6 +112,17 @@ def ls(ctx, opts, owner, page, page_size): | |||
|
|
|||
| print_vulnerability_policies(policies) | |||
|
|
|||
| click.echo() | |||
There was a problem hiding this comment.
does the newline break JSON parsing if you use JSON output?
There was a problem hiding this comment.
Unresolved this one.
There was a problem hiding this comment.
☝️ Still need an answer here
BartoszBlizniak
force-pushed
the
ceng-355-httpsgithubcomcloudsmith-iocloudsmith-clipull171
branch
from
cf3ca3b to
c79c929
Compare
cloudsmith_cli/cli/utils.py
Outdated
| current_page = page | ||
| while True: | ||
| page_results, page_info = api_function( | ||
| page=current_page, page_size=page_size, **kwargs |
There was a problem hiding this comment.
If we're in the "show all" block, we should probably disregard the page_size that was passed into the function and hard code {{whatever the maximum page_size is}} to minimise chattiness.
Do we have a MAX_PAGE_SIZE constant or equivalent in this project already that we might reuse? 🤔
There was a problem hiding this comment.
☝️ We should do this - reach out if you need more 👀
| @@ -110,6 +112,17 @@ def ls(ctx, opts, owner, page, page_size): | |||
|
|
|||
| print_vulnerability_policies(policies) | |||
|
|
|||
| click.echo() | |||
There was a problem hiding this comment.
Unresolved this one.
|
Code Climate has analyzed commit ce41ea2 and detected 10 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 87.5% (50% is the threshold). This pull request will bring the total coverage in the repository to 73.4% (1.1% change). View more on Code Climate. |
cloudsmith_cli/cli/validators.py
Outdated
| def validate_show_all(ctx, param, value): | ||
| """Ensure that --show-all is not used with --page (-p) or --page-size (-l).""" | ||
| if value: | ||
| # Check if either page or page_size parameters were provided, regardless of value | ||
| if any(param in ctx.params for param in ["page", "page_size"]): | ||
| raise click.UsageError( | ||
| "The --show-all option cannot be used with --page (-p) or --page-size (-l) options." | ||
| ) | ||
| return value | ||
|
|
||
|
|
There was a problem hiding this comment.
Can you add a test for this please?
Does the order in which the parameters are passed affect the validation?
cloudsmith_cli/cli/utils.py
Outdated
| current_page = page | ||
| while True: | ||
| page_results, page_info = api_function( | ||
| page=current_page, page_size=page_size, **kwargs |
There was a problem hiding this comment.
☝️ We should do this - reach out if you need more 👀
| @@ -110,6 +112,17 @@ def ls(ctx, opts, owner, page, page_size): | |||
|
|
|||
| print_vulnerability_policies(policies) | |||
|
|
|||
| click.echo() | |||
There was a problem hiding this comment.
☝️ Still need an answer here
…71' of github.com:cloudsmith-io/cloudsmith-cli into ceng-355-httpsgithubcomcloudsmith-iocloudsmith-clipull171
This PR includes the suggestion/contribution from PR 171 (Issue 170)
--show-allflag to all supported pagination elements (Repo, Package, Upstream, Policy)slug_permto documentation for dependency command for clarity