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

Fix Consumer.fromJavaConsumer and Producer.fromJavaProducer type signatures #1033

Merged
merged 5 commits into from
Aug 30, 2023

Conversation

devsprint
Copy link
Contributor

Found the issue while trying to wrap the kafka Consumer and Producer with KafkaTelemetry to integrat Open Telemetry for tracing.

Added a pair of convenient methods.

…mer that rely on interface instead of implementation
@guizmaii
Copy link
Member

@devsprint What's the diff with the functions just above yours? 🤔

@devsprint
Copy link
Contributor Author

Just rely on the interface instead of implementation

@devsprint
Copy link
Contributor Author

Not sure that is the best approach. maybe we can have a single method that use the interface as parameter instead of the implementation.

@devsprint devsprint closed this Aug 30, 2023
@devsprint devsprint reopened this Aug 30, 2023
@guizmaii
Copy link
Member

Just rely on the interface instead of implementation

Can you instead fix the previous functions? It's not good to rely on the implementation. The interfaces should have been used

@devsprint
Copy link
Contributor Author

yes. I will update the branch in few minutes.

@guizmaii
Copy link
Member

(BTW, I'm fixing the CI here: #1032. I'll merge my PR as soon as the CI passes)

@guizmaii
Copy link
Member

@devsprint I fixed the CI in the master branch (sorry about that). Can you rebase your branch, please?

@guizmaii guizmaii changed the title feat: added new integration points with java kafka producer and consumer Fix Consumer.fromJavaConsumer and Producer.fromJavaProducer type signatures Aug 30, 2023
@guizmaii guizmaii merged commit 87a4358 into zio:master Aug 30, 2023
@guizmaii
Copy link
Member

Thanks @devsprint!

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.

3 participants