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

Kafka: authenticate with Event Hubs using OAuth2 #58

Merged
merged 1 commit into from
Feb 16, 2024

Conversation

spenes
Copy link
Contributor

@spenes spenes commented Feb 15, 2024

#57

Copy link
Contributor

@istreeter istreeter left a comment

Choose a reason for hiding this comment

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

I think this is completely fine.

My only comment is... previously when I've seen ClassTag used, it has always been passed implicitly. So something like:

  def resource[F[_]: Async, T <: AzureAuthenticationCallbackHandler: ClassTag](
    config: KafkaSinkConfig
  ): Resource[F, Sink[F]] = {

Then in your loader it would look like:

override def badSink: SinkProvider = KafkaSink.resource[F, SinkAuthHandler](_)

To be honest, I'm not 100% certain whether that is the convention to pass ClassTags implicitly. Maybe it's just the examples I've seen.

@spenes
Copy link
Contributor Author

spenes commented Feb 15, 2024

Thanks @istreeter! Actually, I tried that but realized that compiler doesn't enforce type parameter to be passed. So, it can be called like this still:

override def badSink: SinkProvider = KafkaSink.resource(_)

In that case, it gives following error while trying to initiate Kafka consumer/producer:

Could not instantiate class scala.runtime.Nothing

It works fine if type parameter is given. However, I think it is prone to obscure bugs since we don't get compiler error if type parameter isn't passed. Therefore, I decided to pass ClassTag explicitly instead.

@istreeter istreeter self-requested a review February 15, 2024 19:25
@spenes spenes merged commit 4c4b5d7 into develop Feb 16, 2024
7 checks passed
@istreeter istreeter deleted the feature/azure-oauth branch March 16, 2024 19:38
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