Discussion on API response changes when destroying certain Katello objects

Development
,
1

Hi! I’m looking at implementing a small API change for Katello involving modifying the responses when destroying / DELETE-ing the following object types:

  • Alternate Content Sources
  • Content Credentials
  • Environments
  • Host Collections
  • Host Subscriptions
  • Repositories

I’m suggesting this change as a potential fix for 36346.

The objects above all implement respond_for_destroy, which in the past would destroy the object and return an empty brace pair when called via the API. I would like to change this response to return the deleted object (as if running a GET request). This will only impact the returned string for successful destroys.

Old response for successful DELETE request on ACS: {}
New response: {"name":"DeleteMe","alternate_content_source_type":"simplified","content_type":"yum", ...}

Does this suggested change impact anyone? Any thoughts or concerns? Thanks!

1 Like
2

I feel like it shouldn’t hurt anyone to suddenly return something when we were returning nothing. The question I’m curious about is if we’re breaking any standards by introducing these changes without a warning. We usually only do deprecation warnings from what I recall.

3

Thanks Ian; glad to hear that. I’m also unsure about whether this change requires a warning.

1 Like
4

A DELETE is implementing RFC 7231: Hypertext Transfer Protocol (HTTP/1.1): Semantics and Content and that states:

If a DELETE method is successfully applied, the origin server SHOULD
send a 202 (Accepted) status code if the action will likely succeed
but has not yet been enacted, a 204 (No Content) status code if the
action has been enacted and no further information is to be supplied,
or a 200 (OK) status code if the action has been enacted and the
response message includes a representation describing the status.

Returning an empty response is unusual. I would expect a HTTP 204 No Content and actually no content (not even {}) if it did delete something.

To me the original issue doesn’t properly describe why the author does expect some result and I would bounce that back.

1 Like
5

We have some other destroy actions that do return the destroyed object, which is what brought up this issue in the first place. The inconsistency in the returns from the API in Katello pointed to having the API return the destroyed object everywhere.

What does Foreman do in this case? If you destroy a host, are the host’s details returned?

6

That’s a good question. This is the relevant code:

Looking at the apidoc generated for 3.4 it does return the deleted object, which is surprising to me:

https://apidocs.theforeman.org/foreman/3.4/apidoc/v2/hosts/destroy.html

Then in 3.5 it returns a 404, which is is also weird and probably a bug in how we generate them:

https://apidocs.theforeman.org/foreman/3.5/apidoc/v2/hosts/destroy.html

1 Like
7

The return HTTP codes are okay. As Ian already pointed out, the reason for this change was consistency in the content being returned on successful delete. With other entities (like architecture, activation_key, host and others) we usually get the details of the deleted object. So I supposed we just missed that detail for ACSes.

1 Like
8

Consistency is great and I’d say it’s more important than the convention in standards. Note the RFC uses SHOULD (using RFC2119) so it doesn’t violate the RFC.

1 Like
9

From what I understand we’re being consistent and are within the standards. As of my PR we’re still returning correct HTTP status codes, just the response body itself has changed. I believe this is ok - DELETE on Mozilla docs.

1 Like