Project

General

Profile

Bug #4181

V2: Api docs in foreman should not specify a root node for POST/PUT

Added by David Davis about 8 years ago. Updated over 7 years ago.

Status:
Rejected
Priority:
Normal
Assignee:
Category:
API
Target version:
-
Difficulty:
Triaged:
No
Bugzilla link:
Fixed in Releases:
Found in Releases:

Description

See the screenshot. On the left is v2 spec and on the right is the actual apidoc from foreman v2 api.


Related issues

Related to Foreman - Bug #4180: V2: The foreman V2 api doesn't support form dataClosed2014-01-24
Related to Foreman - Bug #6248: Some JSON responses are still nested in a root nodeClosed2014-06-17
Related to Foreman - Bug #5178: Users API requires parameters to be wrappedClosed2014-04-14

History

#1 Updated by Dominic Cleal about 8 years ago

  • Category set to API

#2 Updated by Adam Price almost 8 years ago

  • Priority changed from Normal to High

#3 Updated by Joseph Magen over 7 years ago

David,

The rails default is to return a blank response for PUT and DELETE. Foreman modified the method `api_behavior` in ActionController::Responder to also return object on PUT and DELETE using the rails code

display resource

See app/controllers/api/api_responder.rb

This code is currently used by all POST, PUT, and DELETE requests.

Note that ONLY GET requests use the RABL templates. Return response of POST, PUT, and DELETE did NOT use RABL.

Since this is just for documentation reasons, I suggest to use the Rails standard as is and to change the documentation accordingly.

In short, PUT/POST/DELETE can receive json with OR without a root, but the response will with WITH root node. However, on the GET, the response is WITHOUT rootnode since we configured the RABL templates to do so.

#4 Updated by David Davis over 7 years ago

  • Priority changed from High to Low

Joseph,

First of all, this has nothing to do with what's being returned. This has do with the data that's being sent to create/update. So I'm not sure I understand the first part of your comment.

Regarding the second part of your comment, the problem is that the api doc (and thus apipie/hammer requests) is not consistent as in certain parts of the fortello API such as organization, there's no root node whereas other parts of the api such as domain, there is. I think that'll confuse users and create problems for us developers.

This is only documentation so I don't think it's a high priority. However, the apipie (and thus hammer requests, etc) use this documentation so it affects the formats of the requests that are being sent to/from katello and foreman. It's caused issues in the past like http://projects.theforeman.org/issues/4180 was difficult to solve as only half the requests (the un-nested ones) were failing.

I still think we need this but it's not a high priority.

#5 Updated by David Davis over 7 years ago

  • Related to Bug #4180: V2: The foreman V2 api doesn't support form data added

#6 Updated by David Davis over 7 years ago

  • Priority changed from Low to Normal

#7 Updated by Joseph Magen over 7 years ago

  • Status changed from New to Ready For Testing
  • Assignee set to Joseph Magen
  • Target version set to 1.8.1

#8 Updated by Dominic Cleal over 7 years ago

  • Subject changed from V2: Api docs in foreman should not require a root node for POST/PUT to V2: Api docs in foreman should not specify a root node for POST/PUT
  • Status changed from Ready For Testing to New
  • Assignee deleted (Joseph Magen)
  • Target version deleted (1.8.1)

Resetting this, because the pull request mentioned changes the response from the API and doesn't change the documentation, as David said. The apipie description of the APIv2 actions includes a wrapped node, which in Foreman's API is optional and as per the documentation (in the screenshot provided) is not used.

#9 Updated by Dominic Cleal over 7 years ago

  • Related to Bug #6248: Some JSON responses are still nested in a root node added

#10 Updated by Dominic Cleal over 7 years ago

  • Related to Bug #5178: Users API requires parameters to be wrapped added

#11 Updated by David Davis over 7 years ago

  • Status changed from New to Assigned
  • Assignee set to David Davis

#12 Updated by Joseph Magen over 7 years ago

  • Assignee changed from David Davis to Joseph Magen
  • Target version set to 1.8.0

#13 Updated by The Foreman Bot over 7 years ago

  • Status changed from Assigned to Ready For Testing
  • Pull request https://github.com/theforeman/foreman/pull/1601 added
  • Pull request deleted ()

#14 Updated by Dmitri Dolguikh over 7 years ago

  • Target version changed from 1.8.0 to 1.7.5

#15 Updated by David Davis over 7 years ago

I think we can close this out.

#16 Updated by Dominic Cleal over 7 years ago

Why?

#17 Updated by David Davis over 7 years ago

I thought from our conversation yesterday we were just leaving the API as is basically and fixing bugs?

#18 Updated by Joseph Magen over 7 years ago

  • Status changed from Ready For Testing to Closed

it was decided to keep api V2 POST/PUT as wrapped parameters

#19 Updated by Dominic Cleal over 7 years ago

  • Status changed from Closed to Rejected
  • Target version deleted (1.7.5)

Also available in: Atom PDF