-
Notifications
You must be signed in to change notification settings - Fork 186
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
Add C8 REST guidelines #4580
base: main
Are you sure you want to change the base?
Add C8 REST guidelines #4580
Conversation
👋 🤖 🤔 Hello, @christinaausley! Did you make your changes in all the right places? These files were changed only in versioned_docs/version-8.6/. You might want to duplicate these changes in docs/.
You may have done this intentionally, but we wanted to point it out in case you didn't. You can read more about the versioning within our docs in our documentation guidelines. |
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.
This is great, and I love it. None of my feedback is blocking.
|
||
The API version number does not match the product version (8.x.x). An API’s version is rather defined by the API version number and the product version, e.g. “_POST /v2/user-tasks/search_ in Camunda 8.7.0”. | ||
|
||
We do API versioning rather than endpoint versioning, i.e., the version changes for all endpoints if there is a breaking change in at least one endpoint. Multiple versions of an API can exist in a product version to allow for a migration period, e.g. “POST /v2/user-tasks/search and POST /v3/user-tasks/search in Camunda 8.7.0”. |
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.
non-blocking question that has nothing to do with this PR, and is for my education: is there a v3 coming in 8.7.0, or is this just an example?
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.
Stopping by to add if this is an arbitrary example, please just say "in a future Camunda version" or something less specific.
|
||
## Data fetching | ||
|
||
Most resources provide at least one endpoint to fetch related data. Most of those endpoints provide data with near-real time consistency that is queried from exported records, if records for the respective resource are exported by the engine. If resources are not based on exported records, e.g. license data or topology information, the data returned by those endpoints can reflect real time insights or static content. |
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.
non-blocking: is there anything to say about the ones that fall outside of the "most"? Is there a general statement we can make about why it is not "all"?
|
||
The **sort** object specifies which fields of the object should be sorted and whether they are sorted in ascending (ASC) or descending (DESC) order. | ||
|
||
The **page** object details how to slice the result set. An initial search request can omit the page object or define the _limit_. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned _firstItemSortValues_ and _lastItemSortValues_ of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes _searchAfter_ or _searchBefore_. |
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.
This is really helpful, actually...I was going to ask you a question about these fields today anyway. 👍
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.
@tmetzke stopping back after attempting to add spec descriptions for these properties, armed with my new understanding -- one other field that isn't mentioned here is from
, an int32.
- Am I correct in thinking
from
is probably an index to start from? e.g.from: 11
means start at the 11th item. - If I am correct, is that a zero-based index or 1-based?
- Do you think we should describe that here, since we covered the other fields?
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.
Potentially blocking:
Since these are fields, I think they should be code
instead of italic. @christinaausley can probably confirm, but that's how I interpret the code blocks section of the style guide.
The **page** object details how to slice the result set. An initial search request can omit the page object or define the _limit_. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned _firstItemSortValues_ and _lastItemSortValues_ of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes _searchAfter_ or _searchBefore_. | |
The **page** object details how to slice the result set. An initial search request can omit the page object or define the `limit`. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned `firstItemSortValues` and `lastItemSortValues` of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes `searchAfter` or `searchBefore`. |
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.
Also blocking:
The properties in the spec are named firstSortValues
and lastSortValues
.
The **page** object details how to slice the result set. An initial search request can omit the page object or define the _limit_. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned _firstItemSortValues_ and _lastItemSortValues_ of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes _searchAfter_ or _searchBefore_. | |
The **page** object details how to slice the result set. An initial search request can omit the page object or define the _limit_. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned _firstSortValues_ and _lastSortValues_ of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes _searchAfter_ or _searchBefore_. |
id: "apis-tools/camunda-api-rest/specifications/get-cluster-topology", | ||
label: "Get cluster topology", | ||
className: "api-method get", | ||
id: "apis-tools/camunda-api-rest/specifications/pin-internal-clock-alpha", |
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.
non-blocking, no action requested, informative comment:
Thanks for doing this reordering!
You might be wondering why you had to do it in two places (here, and in the top-level version-8.6-sidbars.json). As tracked in #3027, this particular sidebar.js file is unused by docusaurus, because numbered versions use a static .json
file for the top-level sidebars. This file only exists in the 8.6 docs because docusaurus copied it over during versioning, and we never deleted it.
All that to say, any changes in this file will just be ignored. If anything, we should probably delete this file....but that is not your responsibility.
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.
After I completed my review, I noticed a couple other adjustments that I think we should make. I'm not 100% certain though, so I'm marking this as "commented" rather than "request changes," with hopes that a tech writer can straighten me out.
|
||
The **sort** object specifies which fields of the object should be sorted and whether they are sorted in ascending (ASC) or descending (DESC) order. | ||
|
||
The **page** object details how to slice the result set. An initial search request can omit the page object or define the _limit_. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned _firstItemSortValues_ and _lastItemSortValues_ of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes _searchAfter_ or _searchBefore_. |
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.
Potentially blocking:
Since these are fields, I think they should be code
instead of italic. @christinaausley can probably confirm, but that's how I interpret the code blocks section of the style guide.
The **page** object details how to slice the result set. An initial search request can omit the page object or define the _limit_. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned _firstItemSortValues_ and _lastItemSortValues_ of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes _searchAfter_ or _searchBefore_. | |
The **page** object details how to slice the result set. An initial search request can omit the page object or define the `limit`. This specifies the maximum number of results to retrieve per request. Subsequent requests can use the value of the returned `firstItemSortValues` and `lastItemSortValues` of the [search responses](#search-responses) to page through the items by copying that array into one of the attributes `searchAfter` or `searchBefore`. |
|
||
| Operator | Syntax | Description | | ||
| -------- | ----------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| $eq | field: { "$eq": value } | Filter where field is equal to value. Abbreviated form field: value is also allowed. | |
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.
| $eq | field: { "$eq": value } | Filter where field is equal to value. Abbreviated form field: value is also allowed. | | |
| `$eq` | `field: { "$eq": value }` | Filter where field is equal to value. Abbreviated form field: value is also allowed. | |
Potentially blocking:
Again, this is my interpretation of the style guide, but I think these bits should all be inline code. (Applies also to many lines that follow this one.)
I noticed a couple things after approving.
Thanks for the elaborate feedback, @pepopowitz 👍 |
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'm a little confused about the audience for this content. It often reads like something that is only relevant for Camundi and should be tucked into the howtos.
Can you help me understand the intention here?
- 200 OK | ||
- 204 No Content | ||
- 400 Bad Request | ||
- Generic error that contains further description in the problem detail. | ||
- 401 Unauthorized | ||
- The client is not authenticated yet. | ||
- The client should try again with a modified Authorization header. | ||
- 403 Forbidden | ||
- The client has incorrect or insufficient permissions for the request. | ||
- 404 Not Found | ||
- 409 Conflict | ||
- The request is trying to modify a resource that is currently not in the right state. | ||
- 412 Precondition failed | ||
- The client should check the cluster status. | ||
- 429 Rate Limited Exceeded | ||
- The client exceeds a defined limit of requests, e.g. Zeebe signaling backpressure due to more requests than the broker can currently process | ||
- 500 Internal Server Error | ||
- Generic error that contains further description in the problem detail. |
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 recommend turning this into a table.
@tmetzke I made some minor grammatical tweaks and echo the feedback from @pepopowitz.
@akeller @tmetzke This is a good conversation. I know Tobias wants to version these guidelines, but I'm wondering what alternatives we could consider here. Regardless of the placement, I will note that this is a lengthy document. We could create a folder and break this into several sub-pages (perhaps in another issue) as follows: Guidelines (overview)
|
The preview environment relating to the commit 772fc4e has successfully been deployed. You can access it at https://preview.docs.camunda.cloud/pr-4580/index.html |
"filter": { | ||
"assignee": {"$eq": "demo"}, | ||
"candidateGroups": { "$in": ["groupA", "groupB"] }, | ||
"variables" : { | ||
"orderVolume": 10000, | ||
"foo": {"$lt": 500}, | ||
"bar": {"$exists": false} | ||
}, | ||
"$or": [ | ||
{ "priority": {"$eq": "high"}, "dueDate": { "$lt": "<date>" } }, | ||
{ "priority": {"$eq": "low"}, "followUpDate": { "$lt": "<date>" } } | ||
] |
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.
🔧 No $or
yet, add multiple conditions.
"filter": { | |
"assignee": {"$eq": "demo"}, | |
"candidateGroups": { "$in": ["groupA", "groupB"] }, | |
"variables" : { | |
"orderVolume": 10000, | |
"foo": {"$lt": 500}, | |
"bar": {"$exists": false} | |
}, | |
"$or": [ | |
{ "priority": {"$eq": "high"}, "dueDate": { "$lt": "<date>" } }, | |
{ "priority": {"$eq": "low"}, "followUpDate": { "$lt": "<date>" } } | |
] | |
"filter": { | |
"assignee": {"$eq": "demo"}, | |
"candidateGroups": { "$in": ["groupA", "groupB"] }, | |
"variables" : { | |
"orderVolume": 10000, | |
"foo": {"$gt": 100, "$lt": 500}, | |
"bar": {"$exists": false} | |
} |
Description
closes camunda/camunda#24239
When should this change go live?
hold
label or convert to draft PR)PR Checklist
My changes are for an already released minor and are in
/versioned_docs
directory.My changes are for the next minor and are in
/docs
directory (aka/next/
).I included my new page in the sidebar file(s).
I added a redirect for a renamed or deleted page to the .htaccess file.