`POST /api/3.1/servers` does not set `updPending`

[zrhoffman]

This Bug Report affects these Traffic Control components:

• Traffic Ops

Current behavior:

In Traffic Ops API v3, you used to be able to include

  "revalPending": false,

or

  "updPending": true,

in the body of POST /servers or PUT /servers to queue updates. As of #6569, however, the server.reval_pending and server.upd_pending columns do not exist, and including those fields in the POST or PUT body does not have any effect.

Since this is undocumented behavior, it is not necessarily a bug. That said, for anyone depending on that behavior in stable TO API version 3, this is a breaking change.

[ocket8888]

... this is undocumented behavior, ...

The "Request Structure" section of the PUT /servers/{{ID}} documentation lists updPending as a valid payload property, making no mention of it being immutable (because it isn't supposed to be), and in fact the documentation for /servers/{{ID}}/queue_update recommend using that method instead. So I'd say it's far from undocumented behavior.

[rawlinp]

I don't necessarily agree with this note in the docs:

In the vast majority of cases, it is advisable that the PUT method of the servers/{{ID}} endpoint be used instead.

IMO queueing updates should be its own separate action -- being able to create a server in TO with updates already queued does not really buy us much. And for updates, it makes sense to queue updates separately without having to include all the unrelated server fields, because it's way more common to queue updates on a server than it is to actually change fields of a server.

So, while this is technically a regression, I actually think it's better this way. Just my 2 cents.

[jhg03a]

If our input/output objects didn't have to be the same because of CRUD, I'd recommend pulling immutable fields out of modifying object inputs.

[ocket8888]

I made that note a long, long time ago - it's much easier when you're learning to use an API to remember how to use one endpoint than it is to remember what second endpoint you need to use to modify some specific field of responses from that first endpoint. That's why the note is there, but that's probably not really a good enough reason for it to exist. Regardless, though, it's there.

[zrhoffman]

Using /api/3.1/servers/{{ID}} would make sense if it had PATCH support. Even then, if t3c queued updates using PUT /api/3.1/servers/{{ID}}, it re-introduces the race condition that #6569 sought to fix (which is why API v4 does not support it).

So if we reintroduce this functionality in API v3, the docs should include a note like "This is deprecated, PUT /servers/{{ID}}/status POST /servers/{{ID}}/queue_update is the recommended way to do it"

[ocket8888]

s/\/status/\/queue_update/g

[zrhoffman]

At a cachegroup, cdn, or topology level, sure. I just meant at a server level

[zrhoffman]

Oh, server has it also

[mitchell852]

so is this works as expected or needs to be fixed? @zrhoffman

[zrhoffman]

@mitchell852 It does not work as expected but only affects API version 3. We're sort of waiting for API version 3 to go away to close the issue