Skip to content

MINOR: Update 3.8 documentation for Kafka Streams#16265

Merged
lucasbru merged 15 commits intoapache:trunkfrom
lucasbru:3_8_upgrade_guide
Jun 26, 2024
Merged

MINOR: Update 3.8 documentation for Kafka Streams#16265
lucasbru merged 15 commits intoapache:trunkfrom
lucasbru:3_8_upgrade_guide

Conversation

@lucasbru
Copy link
Copy Markdown
Member

All public interface changes should be briefly mentioned in the
upgrade guide.

Committer Checklist (excluded from commit message)

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

Comment thread docs/streams/upgrade-guide.html Outdated
Comment thread docs/streams/upgrade-guide.html Outdated
Comment thread docs/streams/upgrade-guide.html Outdated
@lucasbru lucasbru changed the title MINOR: update 3.8 upgrade guide for Kafka Streams MINOR: Update 3.8 documentation for Kafka Streams Jun 11, 2024
@lucasbru lucasbru requested a review from ableegoldman June 11, 2024 17:01
@lucasbru
Copy link
Copy Markdown
Member Author

@ableegoldman @nicktelford @calmera We need to update the documentation based on your KIPs. Mostly I was able to copy the docs from the KIPs, so instead of pinging everyone individually, I did it myself. Feel free to leave comments.

Comment thread docs/streams/developer-guide/config-streams.html
Copy link
Copy Markdown
Contributor

@nicktelford nicktelford left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @lucasbru

Comment thread docs/streams/developer-guide/config-streams.html Outdated
Comment thread docs/streams/developer-guide/config-streams.html Outdated
@lucasbru lucasbru requested a review from mjsax June 12, 2024 10:25
Comment thread docs/streams/developer-guide/config-streams.html Outdated
Comment thread docs/streams/upgrade-guide.html Outdated
Comment on lines +149 to +150
This change also removes the internal config <code>internal.task.assignor.class</code> that was used for the same
purpose. If you were using this config, you should switch to using <code>task.assignor.class</code> instead.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

note: we haven't removed this config yet, planning to do so after 3.8

Might still be worth mentioning it here to encourage anyone who was using it before to switch over to the new config/the new StickyTaskAssignor interface. So we can keep this wording but just change it from "We removed the internal config..." to something like "We introduced a new config to replace the internal config ..."

Copy link
Copy Markdown
Member

@ableegoldman ableegoldman Jun 12, 2024

Choose a reason for hiding this comment

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

update: we will be renaming the old StickyTaskAssignor class to LegacyStickyTaskAssignor (if/when this PR is merged) so perhaps we should just say something like this:

"If you were using the old internal config internal.task.assignor.class to plug in a task assignor, please migrate your code to the new public config and pass in an assignor that implements the new TaskAssignor interface"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll wait with the documentation updates once the renamings have settled in the 3.8 branch

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@lucasbru alright, just merged that PR to 3.8. Here's where 3.8 stands:

  1. has not removed the old internal.task.assignor config, though we want to encourage anyone using that to migrate to the new public config instead. should say something like "If you were using the old internal.task.assignor.class config, you should switch to using the new task.assignor.class config instead"
  2. renamed the old TaskAssignor interface (which goes with the old internal.task.assignor config) to the LegacyTaskAssignor (don't need to say anything about this in the docs, just adding it for context)
  3. renamed the old StickyTaskAssignor (which implements the LegacyTaskAssignor) to the LegacyStickyTaskAssignor, so "StickyTaskAssignor" now refers to (only) the new version of this assignor which implements the new TaskAssignor interface. For the docs, we should mention something like "If you were previously plugging in the StickyTaskAssignor via the old internal.task.assignor.class config, you will need to make sure that you are importing the new version of the StickyTaskAssignor when you switch over to the new task.assignor.class config"

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I updated the migration guide. Feel free to suggest edits.

Comment thread docs/streams/upgrade-guide.html Outdated
Comment thread docs/streams/developer-guide/config-streams.html Outdated
Comment thread docs/streams/developer-guide/config-streams.html Outdated
Comment thread docs/streams/developer-guide/config-streams.html Outdated
Comment thread docs/streams/upgrade-guide.html Outdated
lucasbru and others added 7 commits June 13, 2024 13:23
Co-authored-by: A. Sophie Blee-Goldman <ableegoldman@gmail.com>
Co-authored-by: A. Sophie Blee-Goldman <ableegoldman@gmail.com>
Co-authored-by: A. Sophie Blee-Goldman <ableegoldman@gmail.com>
Co-authored-by: A. Sophie Blee-Goldman <ableegoldman@gmail.com>
Co-authored-by: Matthias J. Sax <mjsax@apache.org>
Co-authored-by: A. Sophie Blee-Goldman <ableegoldman@gmail.com>
@lucasbru lucasbru requested a review from ableegoldman June 19, 2024 08:40
@lucasbru lucasbru requested a review from mjsax June 19, 2024 08:40
Copy link
Copy Markdown
Member

@ableegoldman ableegoldman left a comment

Choose a reason for hiding this comment

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

Few more minor fixes but in general this is looking good

Comment thread docs/streams/developer-guide/config-streams.html Outdated
Comment thread docs/streams/upgrade-guide.html Outdated
lucasbru and others added 2 commits June 21, 2024 14:34
Co-authored-by: A. Sophie Blee-Goldman <ableegoldman@gmail.com>
Co-authored-by: A. Sophie Blee-Goldman <ableegoldman@gmail.com>
@lucasbru lucasbru requested a review from ableegoldman June 21, 2024 12:34
@lucasbru lucasbru merged commit 7ab7773 into apache:trunk Jun 26, 2024
lucasbru added a commit that referenced this pull request Jun 27, 2024
All public interface changes should be briefly mentioned in the
upgrade guide.

Reviewers: Matthias J. Sax <matthias@confluent.io>, Anna Sophie Blee-Goldman <sophie@responsive.dev>, Nick Telford <nick.telford@gmail.com>
abhi-ksolves pushed a commit to ksolves/kafka that referenced this pull request Jul 31, 2024
All public interface changes should be briefly mentioned in the
upgrade guide.

Reviewers: Matthias J. Sax <matthias@confluent.io>, Anna Sophie Blee-Goldman <sophie@responsive.dev>, Nick Telford <nick.telford@gmail.com>
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.

4 participants