On Tue, Oct 24, 2017 at 12:04:03PM -0500, Adam Krone wrote: > (resending, forgot to remove my signature again...) > > >I wonder how terraform handle other crontab like files? > > Terraform doesn't need to know about implementation details like crontab > files. If any Terraform providers currently handle crontab files, it's > because the API abstracts that. All Terraform needs to know is how to speak > to the API to manage resources, which involves keeping track of an ID for > future requests on that resource. > > We can already get all backup IDs with GET /api2/json/cluster/backup, and > that ID is necessary for any of the /api2/json/cluster/backup/{id} methods, > so it's not like we're exposing information that's not already known. > > As long as the API is allowed to be consumed by third parties, this seems > like an important addition in order to support the widest variety of use > cases. > > Ignoring the backup situation for a moment, I think it would be a good > addition overall to add the resource's current state to the response of all > non-GET methods, instead of just returning null. Granted, the web GUI > wouldn't need to consume this information, however consider the extra bit > of verification it could provide in pvesh. > > When I create a user, for example, pvesh could immediately display the > created user, providing some additional feedback beyond the success status > code. This becomes even more useful in situations where optional parameters > that aren't submitted with the request have default values set by the API. > Perhaps there is a default value that isn't obvious to the user, and having > the immediate feedback might prompt them to update it. > > Of course, this also helps out other third party libraries, like the one > I'm working on, as it gives us more information about the resource in fewer > requests. Instead of creating a resource and then issuing a GET in order to > get the most complete picture of its current state, all that information > could be exposed in the initial create request.
I agree that improving the API schema to include more return value definitions, and returning more stuff sounds like a good idea. note that for some API calls, this will not be possible as we fork a worker and just return the worker ID, and the resulting log needs to be fetched actively by the client (this is something needs to be handled by the automation provider in any case!). the problem with adding default values to such returned states is that most defaults in PVE are implicit, and thus subject to change with future updates. returning those as if they were part of the saved state on the PVE side would be wrong. there are other optional parameters where it would make sense though, as they are actually generated AND saved by PVE (think MAC addresses, volume names, ...). so this is probably a bigger effort across the whole API to check and improve each API path. for the backup crontab issue at hand, it could probably be fixed rather simply by generating a UUID and storing it as a comment in the crontab, next to the actual entry. if no such comment exists, we can use the old ID scheme in order to remain compatible with existing crontabs. does that sound workable for you? would you be willing to attempt to implement it and send a patch (or alternatively, file an enhancement request)? :) _______________________________________________ pve-devel mailing list pve-devel@pve.proxmox.com https://pve.proxmox.com/cgi-bin/mailman/listinfo/pve-devel