Re: [pve-devel] [PATCH manager] fix #2435: GUI: LXC summary: Add privileged status and os type

[Matthias Heiserer <m.heiserer@proxmox.com>]

On 09.09.2022 08:20, Thomas Lamprecht wrote:
> Can you please split this into two commits, first the unpriv one and then the OS type one?
> 
> Am 08/08/2022 um 13:48 schrieb Matthias Heiserer:
>> ---
>>
>> I'm a bit unsure about the icons, there are probably better ones, but I couldn't
>> think of/find any. One alternative for OS type would be the OS logos.
> 
> That'd be nice(r) and make it stand out more. But, while we already got the Debian
> one available (currently used for the apt repo gui), you'd need to assemble the other ones
> yourself, ensuring to only pull the official ones with a compatible license.
> 
> After a quick search I found a project[0] that would seem to provide the desired logos,
> we could pull out the for us relevant one and ship it in manager, or alternatively create
> a full new package with all logos and CSS files exposed, wouldn't be _that_ much more work
> and possible a bit cleaner.
> 
> [0]: https://github.com/lukas-w/font-logos
> 
Sure

>>
>>   www/manager6/.lint-incremental        |  0
>>   www/manager6/panel/GuestStatusView.js | 44 +++++++++++++++++++++++++++
>>   2 files changed, 44 insertions(+)
>>   create mode 100644 www/manager6/.lint-incremental
>>
>> diff --git a/www/manager6/.lint-incremental b/www/manager6/.lint-incremental
>> new file mode 100644
>> index 00000000..e69de29b
>> diff --git a/www/manager6/panel/GuestStatusView.js b/www/manager6/panel/GuestStatusView.js
>> index 8db1f492..96e9adea 100644
>> --- a/www/manager6/panel/GuestStatusView.js
>> +++ b/www/manager6/panel/GuestStatusView.js
>> @@ -11,6 +11,30 @@ Ext.define('PVE.panel.GuestStatusView', {
>>   	};
>>       },
>>   
>> +    controller: {
>> +	xclass: 'Ext.app.ViewController',
>> +
>> +	init: function(view) {
>> +	    const nodename = view.pveSelNode.get("node");
>> +	    const id = view.pveSelNode.get("vmid");
>> +	    const me = this;
>> +	    Proxmox.Utils.API2Request({
>> +		url: `/api2/extjs/nodes/${nodename}/lxc/${id}/config`,
>> +		waitMsgTarget: view,
> 
> I could swear we got the config already available somewhere in the hierarchy there,
> and checking the debug console's network view I saw a config load in the summary
> panel, naturally without your patch applied yet ;-), for the pmxNotes panel.
> 
> I really want to avoid loading the same stuff in the same view multiple times.
> 
> As we don't need the config often in the LXC panels (just Summary and Network,
> otherwise we use the slightly odd /pending endpoint) I'd not add it in the
> PVE.lxc.Config, but rather load it once in the pveGuestSummary
> 
> Then you can pass it along to the notes and status panels for reuse on declaration.
> To avoid forced dependency bump and browser cache issues please keep the widget
> toolkits notes view's config load as fallback.
Correct me if I'm wrong, but we can't pass it to Network (because 
different Tab), and passing to to pmxNotes would result in a circular 
dependency.
I guess we can store it in some global object?
> 
> 
>> +		method: 'GET',
>> +		success: function(response) {
> 
> fyi: if you only ever need to access child properties and you can be sure that they
> exist (normally fine in the "success" callback) you could use some destructuring
> here:
> 
> success: ({ result }) => { ... },
> 
> style wise you could naturally also do:
> 
> success: ({ result: { data: { ostype, unprivileged } } }) => { ... }
> 
> but it increases assumptions about the return type, as you do not check for
> definedness or use nullish coalescing below anyway it would stay at the same
> risk level w.r.t exceptions in this case, and as said, should be fine for the
> success callback (no hard feelings to either variant on my side)
> 
In that case, I'll use the current format without destructuring, I feel 
like it's more readable.