In my opinion, introducing PATCH methods seems like unnecessary complexity
I could buy that. It depends on who you are, the client sees pushing up
immutable or unchanged fields as unnecessary complexity, while a
developer might see maintaining another method as unnecessary when
others suffice. At the least, I'd like to leave in the PATCH handler
commented out so that it can remain as a reference to others, since it
is the only one that exists (to my knowledge). But I'd also point out
that none of the PATCH fields are "nullable", so sending them as `null`
isn't valid and can be safely treated as equivalent to them being
un-set. That is specific to this endpoint (and possibly others), though.
Providing the ability to PUT kind of encourages the idea that you don't really
have to get your invalidations right the first time
I don't really agree with that. Or at least, if it does, then DELETE
does the same thing; you don't need to get it right because you can
delete it if it isn't.
[Providing the ability to PUT kind of encourages the idea]... that you can
just update an existing invalidation job to change the regex instead of
creating a new invalidation with a different regex (when really they should be
created as separate jobs)
maybe, but doesn't everything that can be PUT suffer from this, then? I
don't think that a user's initial reaction to "I need to invalidate
content" will be "I can just re-use old jobs", especially since
old/defunct jobs can now be deleted. Plus, regex isn't the only thing a
user may want to change, suppose they only want to extend or shrink the
duration. In that case, editing a job makes perfect sense, and -
especially in the extension case - deleting does not.
On 7/30/19 4:48 PM, Rawlin Peters wrote:
In my opinion, introducing PATCH methods seems like unnecessary
complexity. We don't really have a good way in TO-Go to support
partial object updates in a holistic manner today, and there are some
difficulties around determining which fields were actually sent by a
client with a null value (e.g. `"foo": null`) vs fields that were
entirely omitted by the client. It would also add to the burden of
testing and maintenance (when a simple PUT implementation would
suffice), and I don't think there's a great way for the TO Go client
to marshal a lib/go-tc struct into a PATCH request that only contains
the fields you'd like to update (sometimes with null/empty values).
As for PUT, I think we could get by with a POST and a DELETE without a
PUT for this particular endpoint, but I'm not sure I really feel
strongly about that. Providing the ability to PUT kind of encourages
the idea that you don't really have to get your invalidations right
the first time, or that you can just update an existing invalidation
job to change the regex instead of creating a new invalidation with a
different regex (when really they should be created as separate jobs).
If you have a bad revalidation deployed, your first priority should
probably be to get rid of it as quickly as possible (via DELETE)
instead of trying to replace it with a different regex (via PUT). In
that case, I'd think it would be advantageous to only provide the
DELETE option instead of both DELETE and PUT. First delete the bad
invalidation ASAP, then work on a better regex.
- Rawlin
On Tue, Jul 30, 2019 at 10:31 AM ocket8888 <[email protected]> wrote:
I have had this PR open for a while:
https://github.com/apache/trafficcontrol/pull/3744
I meant to bring this to the mailing list earlier, but I forgot :P
The reason this merits discussion is that the PR adds several method
handlers to the /jobs endpoint that didn't exist in Perl:
- POST
lets users create new jobs directly at this endpoint. My hope is
that the /user/current/jobs endpoint will fall into disuse, and we can
consolidate some functionality in one place. Obviously, this
necessitates a CDN-wide queue of reval updates.
- PUT
allows jobs to be replaced. This queues reval updates CDN-wide.
- PATCH
allows jobs to be edited. This also queues reval updates CDN-wide
- DELETE
deletes jobs. This, too, queues reval updates CDN-wide
Which I think is a good idea. Without any way to mutate created jobs, a
typo can have dire consequences that can't be taken back. But since
POST->DELETE->POST is really just editing with more steps, a PUT/PATCH
seemed to make sense.
thoughts?
