[pve-devel] [PATCH pve-manager 2/2] pvenode task log: don't hardcode 'limit' in CLI

[Thomas Lamprecht]

On 31.08.21 16:42, Dylan Whyte wrote:
> Removes the hardcoded '--limit' option when setting up the
> 'pvenode task log' command. This allows a user to control this option
> themselves.
> 
> Also changes the default limit from 50 to 500, as this is what we use in
> the GUI, and should be plenty for the majority of tasks.

but certainly not all, and certainly a behavior change for something that is
possibly used in scripts.

> 
> Signed-off-by: Dylan Whyte <d.whyte at proxmox.com>
> ---
>  PVE/API2/Tasks.pm  | 4 ++--
>  PVE/CLI/pvenode.pm | 3 +--
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/PVE/API2/Tasks.pm b/PVE/API2/Tasks.pm
> index 9cd1e56b..ce87cdac 100644
> --- a/PVE/API2/Tasks.pm
> +++ b/PVE/API2/Tasks.pm
> @@ -347,7 +347,7 @@ __PACKAGE__->register_method({
>  	    limit => {
>  		type => 'integer',
>  		minimum => 0,
> -		default => 50,
> +		default => 500,
>  		optional => 1,
>  		description => "The maximum amount of lines that should be printed.",
>  	    },
> @@ -379,7 +379,7 @@ __PACKAGE__->register_method({
>  	my $user = $rpcenv->get_user();
>  	my $node = $param->{node};
>  	my $start = $param->{start} // 0;
> -	my $limit = $param->{limit} // 50;
> +	my $limit = $param->{limit} // 500;

FWIW, we could also differ between the case for when the RPC env is of type CLI and
when not, falling back to different defaults then.

That'd keep the default of 50 lines (as that change could be counted as API change,
and while I'm not rejecting it I'd like at least another opinion on that) and also
the current 100k for the CLI one.

I.e., something like:

my $limit = $param->{limit} // ($rpcenv->{type} eq 'cli' ? 100000 : 50);


One could mayve document that in the param's description.

>  
>  	$convert_token_task->($task);
>  
> diff --git a/PVE/CLI/pvenode.pm b/PVE/CLI/pvenode.pm
> index acef6c3b..b45735e0 100644
> --- a/PVE/CLI/pvenode.pm
> +++ b/PVE/CLI/pvenode.pm
> @@ -194,8 +194,7 @@ our $cmddef = {
>  	    my ($data, $schema, $options) = @_;
>  	    PVE::CLIFormatter::print_api_result($data, $schema, undef, $options);
>  	}, $PVE::RESTHandler::standard_output_options],
> -	# set limit to 1000000, so we see the whole log, not only the first 50 lines by default
> -	log => [ 'PVE::API2::Tasks', 'read_task_log', [ 'upid' ], { node => $nodename, limit => 1000000 }, sub {
> +	log => [ 'PVE::API2::Tasks', 'read_task_log', [ 'upid' ], { node => $nodename }, sub {
>  	    my ($data, $resultprops) = @_;
>  	    foreach my $line (@$data) {
>  		print $line->{t} . "\n";
>