On 8/14/19 12:03 PM, Thomas Lamprecht wrote:
Am 8/6/19 um 2:41 PM schrieb Stefan Reiter:
I don't see a reason to blanket-forbid excluding VMs in pool backups, so I felt 
leaving the API unchanged was the better option in this case. The GUI is the 
broken part, the API is working fine, albeit for a use-case it wasn't 
intentionally designed for.


But then the WebGUI would need to be adapted to cope with such a case,
as currently adding excludes to a pool based backup job results in a
rather strange and wrong visualization, e.g., "All except 1074" here,
but all is not selected and editing the job shows an inconsistent wrong
state too.


That's why I made the patch to the UI, since it is the component that breaks. If the user knows what they are doing, they can still do so. I see your point though.

That said, I'm fine with either change.

I'd rather say we need both, or?

That would be unnecessary, since both patches mentioned here would only lead to deleting the "exclude" data from the update API call. Ignoring external API calls, they basically to the same thing.


"Pool Based" backup jobs should stay simple, and as it does not really
works in all cases I'd rather prevent exclusions with that mode.
People needing to exclude some VMs can just use other modes..


In that case I'd say we go for Tims backend version. I'll send the change as a v2 to make it better to apply.

PS: please avoid "top posting" if possible :)


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

Reply via email to