-
Notifications
You must be signed in to change notification settings - Fork 188
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 message correlation endpoint and Rest API spec #4166
Conversation
Regenerates the Camunda REST API spec according to the latest main branch in the camunda repository.
👋 🤖 🤔 Hello! Did you make your changes in all the right places? These files were changed only in docs/. You might want to duplicate these changes in versioned_docs/version-8.5/.
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.
The PR looks good to me 🚀
We should use an imperative form for the summaries, for example Correlate a message
should be Correlate message
.
I will open a PR next week to adjust the OpenAPI specification, not sure if we can wait to merge this PR and generate the doc again after
docs/apis-tools/camunda-api-rest/specifications/correlate-a-message.api.mdx
Outdated
Show resolved
Hide resolved
docs/apis-tools/camunda-api-rest/specifications/correlate-a-message.api.mdx
Show resolved
Hide resolved
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.
Note to myself that this will need to be updated in Postman upon release 👍
title: "Correlate a message" | ||
description: "Publishes a message and correlates it to a subscription. If correlation is successful it" | ||
sidebar_label: "Correlate a message" |
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.
@christinaausley because these files are all generated from the OpenAPI spec, we should look into making these changes at the source, as well as the spec included in the docs. Otherwise, every time we regenerate the spec, the non-preferred text is going to be reintroduced.
- @christinaausley can you make the associated changes in the api/camunda/camunda-openapi.yaml file. Everywhere that you updated language in a specifications file, you'll want to replicate in that file. This will give us one soft layer of protection from introducing a regression, as we'll see the regression changed in a later PR.
- @remcowesterhoud can you point us at a location to make these kinds of changes in the original spec?
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.
The original spec lives in the Zeebe repository. But as you can see there we do have a full description. Since I've copied that file to the docs we also have the full description here.
I suppose the generation cannot handle multiline strings? 🤔
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 wonder if this is a big issue as it's just the page description, which as far as I know is not listed anywhere. On the page itself all looks good to me.
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 suppose the generation cannot handle multiline strings? 🤔
That's very possible! I'll take a look and see what I learn.
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.
@pepopowitz I've updated the spec accordingly 👍
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.
Welllll this is a significantly longer and more nuanced response than I expected to be writing, but here we go.
My research findings:
The OpenAPI generator that drives our API doc generation uses the description
property on an endpoint in the OpenAPI spec for two things:
- The generated page's body. The OpenAPI generator takes all lines of the
description
property for this bit, as we can see when viewing the body of the page associated with this specific file. - This
description
property in the generated markdown's frontmatter. Docusaurus turns that frontmatter property into the<meta name="description">
tag on the emitted HTML page. The OpenAPI generator takes only the first line of the property for this -- I think this is a reasonable limitation, and we have other examples where it is helpful, because the spec'sdescription
includes notes that we probably don't want in the page's meta description. (Example:camunda-docs/api/camunda/camunda-openapi.yaml
Lines 544 to 551 in d343afc
description: | Search for user tasks based on given criteria. :::note This endpoint is experimental and not enabled on Camunda clusters out of the box. See the [Camunda 8 REST API overview](/apis-tools/camunda-api-rest/camunda-api-rest-overview.md#query-api) for further details. :::
My proposal:
We align our description
properties to this limitation by making the first line a complete sentence and high-level summary of the endpoint. Additional information can come on subsequent lines, but the first line itself should make sense in isolation of the rest of the property. In this specific example, it would be Publishes a message and correlates it to a subscription.
, and the second sentence would begin after a line break.
Where do we make those changes?
Ideally, we would make those changes at the source (the spec in the API project itself), so that we don't introduce regressions every time we re-generate these docs.
However, there may be linting involved in that project that is handling the line breaks automatically. If that's the case, we could make the changes in this project only, and either:
- Add a function to this API's generation strategy that detects the multi-line descriptions and inserts an end of line after the first sentence, or
- Adjust it manually in the OpenAPI spec in this PR, and then count on us noticing an unexpected diff when we re-generate the docs, and fix it.
2 is obviously ripe for human error, so 1 would be my preference....however I'd open a separate issue to take care of that, as I don't think it's urgent.
So what does this PR need to do to move forward?
If my proposal seems reasonable:
- ✅ We adjust multi-line
description
properties in this API's spec within this project, so that there is a line break after the first sentence. Assignee: if anyone wants to do this, go for it, otherwise I'll take care of it tomorrow. - We follow up this PR with a change to the OpenAPI spec in the API's project, to align with this change & Christina's technical review changes, to avoid regressions at next generation. Assignee: me, and @christinaausley if she wants to pair on it.
- If the line-breaks I'm proposing are impossible or rejected in that project, I'll create an issue to add a step to this API's generation strategy to ensure multi-line descriptions start with one complete sentence. Assignee: me.
If people disagree with my proposal, we'll figure something else out!
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.
FYI, I updated the description property in question, as described above, in a new commit on this branch. I also re-generated the docs from the spec.
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.
Awesome, thank you @pepopowitz! Does this mean this is good to merge now?
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 good with it if @christinaausley is!
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.
Ship itttttt 🚢
🧹 Preview environment for this PR has been torn down. |
## Description Incorporates changes into the OpenAPI spec which were proposed in camunda/camunda-docs#4166. This should prevent us in the future from introducing regressions to the docs for these descriptions. Collaboration with @christinaausley. ## Related issues Follow-up from camunda/camunda-docs#4166
## Description Incorporates changes into the OpenAPI spec which were proposed in camunda/camunda-docs#4166. This should prevent us in the future from introducing regressions to the docs for these descriptions. Collaboration with @christinaausley. ## Related issues Follow-up from camunda/camunda-docs#4166
* feat: regenerate Camunda Rest API endpoints Regenerates the Camunda REST API spec according to the latest main branch in the camunda repository. * feat: document the difference between message correlation and message publish * style(formatting): technical review * update spec * docs: update incoming spec to use sentence boundaries in description properties. * chore: regenerate docs from spec --------- Co-authored-by: Christina Ausley <[email protected]> Co-authored-by: Steven Hicks <[email protected]>
Description
Documents the new message correlation feature in Zeebe. It explains the difference between message correlation and publish. It also adds the new endpoint to the camunda rest api (along with other endpoints added to the spec).
The first commit is all auto-generated. The second commit contains my relevant changes 🙂
closes #4164
When should this change go live?
hold
label or convert to draft PR)PR Checklist
/versioned_docs
directory./docs
directory (aka/next/
).