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

Add Topic Id to Metadata Response #4300

Merged
merged 7 commits into from
Aug 7, 2023
Merged

Conversation

pranavrth
Copy link
Member

As part of KIP-516, we need to add support for topic id in all the major APIs. As a prerequisite for KIP-848, we are adding topic id to Metadata Response in this PR. We are incrementing support for v12 for Metadata protocol though we still don't support topic id in Metadata Request for now. That will be added once we implement full support for topic id as part of KIP-516.

This PR includes:

  • Adding new data type UUID required to store topic id.
  • Base64 encoding of UUID
  • Extracting topic id from Metadata Response to internal Metadata struct
  • Some small refactoring

@pranavrth
Copy link
Member Author

There were 2 different places where base64 encoding was done in the code. Both of them are dependent on OpenSSL. I have moved 1 implementation to common place to be used as a generic base64 encoder.

Open points to discuss:

  • Though I have kept it dependent on OpenSSL, we should also make it independent of OpenSSL as topic id should not be dependent on OpenSSL.
  • 2nd place implementation of base64 encoding (in rdkafka_sasl_scram.c) is not changed to use generic implementation extracted in this PR.

@pranavrth pranavrth force-pushed the dev_metadata-response-topic-id branch from 6795ad5 to 1c9e5c2 Compare May 29, 2023 19:23
src/rdkafka_sasl_scram.c Outdated Show resolved Hide resolved
@emasab emasab marked this pull request as ready for review July 18, 2023 14:27
src/rdbase64.c Show resolved Hide resolved
src/rdkafka_buf.h Outdated Show resolved Hide resolved
src/rdbase64.c Outdated Show resolved Hide resolved
src/rdkafka_proto.h Outdated Show resolved Hide resolved
src/rdkafka_proto.h Outdated Show resolved Hide resolved
src/rdkafka_buf.h Outdated Show resolved Hide resolved
src/rdkafka_buf.h Outdated Show resolved Hide resolved
src/rdkafka_proto.h Outdated Show resolved Hide resolved
src/rdkafka_buf.h Outdated Show resolved Hide resolved
src/rdkafka_buf.h Outdated Show resolved Hide resolved
@pranavrth pranavrth force-pushed the dev_metadata-response-topic-id branch 2 times, most recently from b6a7b45 to b32bb78 Compare August 2, 2023 12:54
@pranavrth pranavrth force-pushed the dev_metadata-response-topic-id branch from 5073a8c to 895358a Compare August 2, 2023 16:17
@cla-assistant
Copy link

cla-assistant bot commented Aug 3, 2023

CLA assistant check
All committers have signed the CLA.

emasab added a commit that referenced this pull request Aug 3, 2023
Needs to be updated after #4300
Copy link
Contributor

@emasab emasab left a comment

Choose a reason for hiding this comment

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

Sent some other improvements

CHANGELOG.md Outdated
@@ -1,5 +1,11 @@
# librdkafka v2.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

version 2.2.1 for the moment as it doesn't change public API

Copy link
Member Author

Choose a reason for hiding this comment

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

Keeping 2.3.0 as we don't have plan for 2.2.1 right now.

CHANGELOG.md Show resolved Hide resolved
INTRODUCTION.md Outdated Show resolved Hide resolved
src/rdbase64.c Outdated Show resolved Hide resolved
src/rdbase64.c Outdated Show resolved Hide resolved
src/rdkafka_proto.h Outdated Show resolved Hide resolved
src/rdkafka_proto.h Outdated Show resolved Hide resolved
src/rdkafka_proto.h Outdated Show resolved Hide resolved
src/rdkafka_proto.h Show resolved Hide resolved
src/rdkafka_sasl_oauthbearer_oidc.c Show resolved Hide resolved
Copy link
Contributor

@emasab emasab left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @pranavrth !

@pranavrth pranavrth merged commit 07262c4 into master Aug 7, 2023
1 check passed
@pranavrth pranavrth deleted the dev_metadata-response-topic-id branch August 7, 2023 14:01
emasab added a commit that referenced this pull request Aug 7, 2023
Needs to be updated after #4300
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