On 01.07.21 10:50, Lorenz Stechauner wrote: > changes to v10: > * dropped already applied patches > * added "check" button - the gui now does not automatically send the metadata > request anymore > * removed (visible) content type selector, because there was only one > hard-coded option every time > * added loading mask while the metadata check is in progress > > Lorenz Stechauner (3): > api: nodes: add query_url_metadata method > ui: Utils: change download task format > fix #1710: ui: storage: add download from url button > > PVE/API2/Nodes.pm | 96 ++++++++++ > www/manager6/Utils.js | 2 +- > www/manager6/storage/Browser.js | 8 + > www/manager6/storage/ContentView.js | 262 +++++++++++++++++++++++++--- > 4 files changed, 343 insertions(+), 25 deletions(-) >
applied series, thanks! Did some followups though: - I moved out the download window into its own file, was a bit out of place in the storage content view and adding 200+ lines code is just better down in a separate file, if not really general code to the existing content view components - I found the event logic a bit hard to grasp, reworked it with a view model - Renamed `Check` to `Query URL`, as when trying to think as user I had no idea what `Check` would actually do. - disabled the `Query URL` button when done so successfully, any change in URL or verify state re-enables it, it stays also enabled if the query itself fails, as that could have been a (network) hiccup, where allowing one to requery makes sense from UX POV. - do not invalidate size/mime on verify cert check toggle, makes no sense to me, the last OK queried info will still be valid. Above could have been noticed earlier on review, sorry for that, but as I was pretty swamped to take a closer look and there are others which can review such things I'm so free to not take the whole blame ;-) In general I wonder if we could do the metadata query also client side, we could just do a HEAD request from there which would be much nicer, but there are also cases where an admin may enter an internal URL or the DNS the browsers queries is another than the PVE hosts have, so while I think it would cover most of the actual cases in practice (as even with internal URLs, the admin often access PVE also from an internal network), it's still not always doable. On the other hand, it would drop any remaining concern about doing some http request from the PVE side. just noting for the record, we can always switch to that in the future and sunset or further-restrict the backend querier if we want... _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
