Skip to content

KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect#13915

Merged
C0urante merged 7 commits intoapache:trunkfrom
yashmayya:KAFKA-14930-document-offset-alter-reset-apis
Jun 29, 2023
Merged

KAFKA-14930: Document the new PATCH and DELETE offsets REST APIs for Connect#13915
C0urante merged 7 commits intoapache:trunkfrom
yashmayya:KAFKA-14930-document-offset-alter-reset-apis

Conversation

@yashmayya
Copy link
Copy Markdown
Contributor

@yashmayya yashmayya commented Jun 26, 2023

Screenshot 2023-06-27 at 4 21 07 PM

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@yashmayya yashmayya added docs connect kip Requires or implements a KIP labels Jun 26, 2023
@yashmayya yashmayya requested a review from C0urante June 26, 2023 08:23
Copy link
Copy Markdown
Contributor

@C0urante C0urante left a comment

Choose a reason for hiding this comment

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

Thanks Yash! Have some thoughts. Also, correct me if I'm wrong, but I think the term "endpoint" is generally more fitting here than "REST API".

Comment thread docs/connect.html Outdated
Comment thread docs/connect.html Outdated
Comment thread docs/connect.html Outdated
<li>Offsets management REST APIs (see <a href="https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect">KIP-875</a> for more details):
<ul>
<li><code>GET /connectors/{name}/offsets</code> - get the current offsets for a connector</li>
<li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: for consistency, we should leave out the period from the last sentence of each item

Suggested change
<li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state.</li>
<li><code>DELETE /connectors/{name}/offsets</code> - reset the offsets for a connector. The connector must exist and must be in the stopped state</li>

Also, would it be possible to link to the docs for the PUT /connectors/{name}/stop endpoint when we refer to it here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hm the PUT /connectors/{name}/stop endpoint docs are in the same connect_rest section as these docs and I don't think it is possible to link to individual list items inside the section?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's possible; I'll add some GH suggestions demonstrating how.

Comment thread docs/connect.html Outdated
yashmayya and others added 5 commits June 27, 2023 10:28
Co-authored-by: Chris Egerton <fearthecellos@gmail.com>
Co-authored-by: Chris Egerton <fearthecellos@gmail.com>
Co-authored-by: Chris Egerton <fearthecellos@gmail.com>
Co-authored-by: Chris Egerton <fearthecellos@gmail.com>
…endpoint; add a note on using a null offset to reset the offset for a partition; add a note on request body format differences between source and sink connectors
Comment thread docs/connect.html Outdated
Comment thread docs/connect.html Outdated
Comment thread docs/connect.html Outdated
Comment thread docs/connect.html Outdated
Comment thread docs/connect.html Outdated
Copy link
Copy Markdown
Contributor

@C0urante C0urante left a comment

Choose a reason for hiding this comment

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

I believe I figured out how to link to the docs for individual endpoints; give this a try and LMK

…sets endpoints; break compact JSON request body examples into multi-line JSON
@yashmayya
Copy link
Copy Markdown
Contributor Author

Thanks Chris. I applied your suggestions and checked it out locally; things look good!

Copy link
Copy Markdown
Contributor

@C0urante C0urante left a comment

Choose a reason for hiding this comment

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

Wonderful, thanks Yash!

@C0urante
Copy link
Copy Markdown
Contributor

Given that this change only touches on our HTML docs, I'll merge without waiting for CI.

@C0urante C0urante merged commit 30b087e into apache:trunk Jun 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

connect docs kip Requires or implements a KIP

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants