Skip to content
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

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

sheldor1510
Copy link
Contributor

Copy link
Contributor

@tzumainn tzumainn left a 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.
Copy link
Contributor

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".

Copy link
Contributor Author

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.
Copy link
Contributor

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.

Copy link
Contributor Author

@sheldor1510 sheldor1510 Jul 23, 2024

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.

Copy link
Contributor

@tzumainn tzumainn left a 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
Copy link
Contributor

@tzumainn tzumainn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - thanks!

@tzumainn tzumainn merged commit 505ad7a into CCI-MOC:master Jul 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants