-
Notifications
You must be signed in to change notification settings - Fork 17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated api-ref in esi-leap documentation #171
Conversation
sheldor1510
commented
Jul 23, 2024
- #566
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty good! Just a few minor comments.
@@ -10,58 +10,82 @@ The Offer API endpoint can be reached at /v1/offers. | |||
* This value will default to returning offers with status 'available'. | |||
* This value can be set to 'any' to return offers without filtering by status. | |||
* resource_uuid: Returns all offers with given resource_uuid. | |||
* resource_type: Returns all offers with given resource_type | |||
* resource_type: Returns all offers with given resource_type. | |||
* resource_class: Returns all offers with given resource_class. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth mentioning here (and below) that this will default to the configured "default resource class", which by default is "ironic_node".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioned.
``` | ||
}' | ||
``` | ||
* The /v1/offers/\<uuid>/claim endpoint supports POST requests to claim the offer with the given uuid. The body of the request contains the lease information with the given values. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would format this section so there's an actual header, so it's more apparent that there's another API function here. Right now it's a bit lost within the POST
section. You can take a look at https://docs.openstack.org/api-ref/baremetal/ for an example of how specialized API commands are listed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added headers to each endpoint to make it structured in a similar way to the Baremetal API documentation. Let me know if this is fine or too much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, that looks good! One last request: can you squash your commits together? We like doing that because it unclutters the commit history.
add information about default resource_type + fixed formatting fixed formatting issue
b30b6a3
to
fea2b37
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - thanks!