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

[Philipp Hufnagl]

On 9/26/23 12:59, Thomas Lamprecht wrote:
> 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 at 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();
>>      },
>>  });
> 

Sorry for the issues. This was my first larger feature. I will
try to improve next time!