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/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)

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.

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


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