Re: [pve-devel] [PATCH manager v8 2/2] fix #4849: ui: download to storage: automatically dectect and configure compression

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 21/09/2023 um 15:09 schrieb Philipp Hufnagl:
> extends the download iso prompt with a "compression algorithm" drop down
> under advanced. User can configure there if a decompression algorithm
> should be used from the storage backend. The compression algorithm will
> be automatically guessed when calling query_url_metadata
> 
> Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
> ---
>  www/manager6/Makefile                       |  1 +
>  www/manager6/form/DecompressionSelector.js  | 13 +++++++++++++
>  www/manager6/window/DownloadUrlToStorage.js | 14 +++++++++++++-
>  3 files changed, 27 insertions(+), 1 deletion(-)
>  create mode 100644 www/manager6/form/DecompressionSelector.js
> 
> diff --git a/www/manager6/Makefile b/www/manager6/Makefile
> index 59a5d8a7..87e66ece 100644
> --- a/www/manager6/Makefile
> +++ b/www/manager6/Makefile
> @@ -34,6 +34,7 @@ JSSRC= 							\
>  	form/ContentTypeSelector.js			\
>  	form/ControllerSelector.js			\
>  	form/DayOfWeekSelector.js			\
> +	form/DecompressionSelector.js			\
>  	form/DiskFormatSelector.js			\
>  	form/DiskStorageSelector.js			\
>  	form/FileSelector.js				\
> diff --git a/www/manager6/form/DecompressionSelector.js b/www/manager6/form/DecompressionSelector.js
> new file mode 100644
> index 00000000..abd19316
> --- /dev/null
> +++ b/www/manager6/form/DecompressionSelector.js
> @@ -0,0 +1,13 @@
> +Ext.define('PVE.form.DecompressionSelector', {
> +    extend: 'Proxmox.form.KVComboBox',
> +    alias: ['widget.pveDecompressionSelector'],
> +    config: {
> +	deleteEmpty: false,
> +    },
> +    comboItems: [
> +		['__default__', Proxmox.Utils.NoneText],
> +		['lzo', 'LZO'],
> +		['gz', 'GZIP'],
> +		['zst', 'ZSTD'],
> +    ],
> +});

For such small things, that have no other use sites, I normally
prefer to just define the values inline..

And finding other use sites for such things is not trivial, as that
would mean we will couple the values that both sites can understand,
which for compression might not be the case (e.g., in the future we
might want to support downloading bz2 here, but not want to support
it for backups).

> diff --git a/www/manager6/window/DownloadUrlToStorage.js b/www/manager6/window/DownloadUrlToStorage.js
> index 90320da4..36ad13fa 100644
> --- a/www/manager6/window/DownloadUrlToStorage.js
> +++ b/www/manager6/window/DownloadUrlToStorage.js
> @@ -66,6 +66,7 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
>  		params: {
>  		    url: queryParam.url,
>  		    'verify-certificates': queryParam['verify-certificates'],
> +		    'detect-compression': view.content === 'iso' ? 1 : 0,
>  		},
>  		waitMsgTarget: view,
>  		failure: res => {
> @@ -84,6 +85,7 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
>  			filename: data.filename || "",
>  			size: (data.size && Proxmox.Utils.format_size(data.size)) || gettext("Unknown"),
>  			mimetype: data.mimetype || gettext("Unknown"),
> +			compression: data.compression || '__default__',
>  		    });
>  		},
>  	    });
> @@ -203,6 +205,17 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
>  			change: 'setQueryEnabled',
>  		    },
>  		},
> +		{
> +		    xtype: 'pveDecompressionSelector',
> +		    name: 'compression',
> +		    fieldLabel: gettext('Decompression algorithm'),
> +		    allowBlank: true,
> +		    hasNoneOption: true,
> +		    value: '__default__',
> +		    cbind: {
> +			hidden: get => get('content') !== 'iso',
> +		    },
> +		},
>  	    ],
>  	},
>  	{
> @@ -223,7 +236,6 @@ Ext.define('PVE.window.DownloadUrlToStorage', {
>  	if (!me.storage) {
>  	    throw "no storage ID specified";
>  	}
> -

please avoid unrelated change in the future.

>  	me.callParent();
>      },
>  });