[pve-devel] API Return Values

[Fabian Grünbichler]

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)? :)