Re: [pdm-devel] [PATCH datacenter-manager 1/5] server: add an optional 'web-url' property for remotes

[Dominik Csapak <d.csapak@proxmox.com>]

On 1/23/25 09:48, Thomas Lamprecht wrote:
> Am 16.01.25 um 09:04 schrieb Dominik Csapak:
>> On 1/15/25 16:12, Thomas Lamprecht wrote:
>>> Am 10.01.25 um 11:21 schrieb Dominik Csapak:
>>>> this contains two optional values to generate a URL for the UI:
>>>> * per_node_template: a 'per node' template to generate an url, used for
>>>>     all places where we have node information. If not set we use the
>>>>     base_url as fallback to generate the url.
>>>>
>>>> * base_url: An URL for reaching the remote. This is e.g. useful if there
>>>>     is some reverse proxy or similar between the users browser and the
>>>>     remote.
>>>
>>> Why two, and not just one? And why was this already applied without
>>> any comment/review from people in the discussion of that feature?
>>
>> while I started out with that, I quickly noticed that in some places/situations we don't
>> have *any* information about the nodes at all, so we always have to have
>> some general base url we can fall back to.
>>
>> (i tried to convey that, but just realized that it was only in the cover letter,
>> should have said it in the commit message too, mea culpa)
> 
> Already talked off-list, but for that we want to add a cache with current
> online nodes where we then could select the first available one with stable
> sorting for some determinism in such a case.
> 
>>
>> we could still have a radio button/checkbox but we always have to have the
>> general url field, and simply turning on and off a single field is more
>> clutter than the current solution.
>>
>> (I'm fine with doing either though if you find that better)
>>
>> Of course, if we decide to implement more functions/features/options here that
>> require more/different inputs, some kind of selection for that would be
>> necessary.
> 
> Can we for now drop the templating and rework this to a single, fixed
> URL? That already helps in a lot of use cases, be it for when the UI is
> only accessible through a reverse proxy from the end user POV or even just
> a different domain for external access compared to how PDM talks with a
> node, as then one can just use a designated node's URL, while that's not
> perfect in case of that node being down, but when not it might be slightly
> nicer than different node URLs as one does not need to login so often.
> 
> With that I could cut a release soonish, we can then improve on this in
> the future.
> When we (re-)add templating and having the node map/status cache, it might
> be even nicer UX to always go through the first available node, even if that
> isn't the node the resource resides on – that would reduce needs for logins,
> i.e. when opening the UI for different guests on different nodes but on the
> same cluster, and also have slightly better caching behavior when loading
> the web UI code and assets. Allowing the PDM user to configure a priority
> order for nodes could then make it really flexible enough; but just some
> thought, not relevant for just now.
> 

Makes sense to me, so i sent a patch that removes it from the config for now:

https://lore.proxmox.com/pdm-devel/20250123132754.3271555-1-d.csapak@proxmox.com/

we can discuss how we want to handle that when we have the cache

> 
>>> The unrelated fixes would be also nice as separate patches, or at
>>> least upfront – as quite often stated already to all devs..
>>
>> true, but to be fair, the UI cleanups were 'upfront' of the other UI patches,
>> just not before the server patch, so they could have been applied either way.
> 
> Hmm, ok, while I'd still slightly prefer that (unrelated) clean-ups/fixes
> happen in a separate series or "fully" upfront, your way here is manageable
> too.

It's not a problem, I'll try to send such cleanups as separate series in the future if possible.
If another series depends on them, I'll add a note in the cover-letter (or similar).


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel