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

[Thomas Lamprecht]

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.


>> 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.