RdKafka -> Confluent.Kafka #2

mhowlett · 2016-11-11T17:57:33Z

ewencp · 2016-11-11T18:12:05Z

Looks like there are still some RdKafka references that aren't for the underlying librdkafka:

doc/template/partials/navbar.tmpl.partial:         <a class="navbar-brand" href="{{_rel}}"><span class="dotnet">RdKafka .NET</span></a>
examples/Misc/Program.cs:            Console.WriteLine($"Hello RdKafka!");
src/Confluent.Kafka/Consumer.cs:            Init(RdKafkaType.Consumer, cfgPtr, config.Logger);
src/Confluent.Kafka/Handle.cs:        internal void Init(RdKafkaType type, IntPtr config, Config.LogCallback logger)
src/Confluent.Kafka/IDeliveryHandler.cs:    /// <remarks>Methods of this interface will be executed in an RdKafka-internal thread and will block other operations - consider this when implementing.</remarks>
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:    enum RdKafkaType
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:        internal static SafeKafkaHandle Create(RdKafkaType type, IntPtr config)
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Could not retrieve metadata");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to query watermark offsets");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to get watermark offsets");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to subscribe to topics");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to unsubscribe");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to close consumer");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to get assignment");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to get subscription");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to assign partitions");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to commit offsets");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to commit offsets");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to fetch committed offsets");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to fetch position");
src/Confluent.Kafka/Internal/SafeKafkaHandle.cs:                throw RdKafkaException.FromErr(err, "Failed to fetch group list");
src/Confluent.Kafka/Library.cs:    /// Miscellaneous APIs for the RdKafka library itself.
src/Confluent.Kafka/Library.cs:        /// Since RdKafka handle deletion is an async operation the
src/Confluent.Kafka/Producer.cs:            Init(RdKafkaType.Producer, cfgPtr, config.Logger);
src/Confluent.Kafka/Producer.cs:                    RdKafkaException.FromErr(
src/Confluent.Kafka/RdKafkaException.cs:    public class RdKafkaException : Exception
src/Confluent.Kafka/RdKafkaException.cs:        public RdKafkaException(string message, ErrorCode errorCode)
src/Confluent.Kafka/RdKafkaException.cs:        internal static RdKafkaException FromErr(ErrorCode err, string message)
src/Confluent.Kafka/RdKafkaException.cs:                return new RdKafkaException(errorMessage, err);
src/Confluent.Kafka/RdKafkaException.cs:                return new RdKafkaException($"{message} ({errorMessage})", err);
src/Confluent.Kafka/Topic.cs:        /// <remarks>Methods of <paramref name="deliveryHandler"/> will be executed in an RdKafka internal thread and will block other operations - consider this when implementing IDeliveryHandler.
src/Confluent.Kafka/Topic.cs:        /// <remarks>Methods of <paramref name="deliveryHandler"/> will be executed in an RdKafka-internal thread and will block other operations - consider this when implementing IDeliveryHandler.
src/Confluent.Kafka/project.json:        "RdKafka.Internal.librdkafka": "0.9.2-ci-28"

Some are definitely user-facing (e.g. RdKafkaException, docs stuff, examples). Might make sense to get git grep RdKafka | grep -v LibRdKafka to provide clean output so we don't have any leftover confusing references.

mhowlett · 2016-11-11T18:13:28Z

@ewencp yeah - on the todo list is to think very carefully about exceptions. Some are direct wrappers around errors from librdkafka, others are not. for the former, the naming RdKafkaException may be more appropriate. I decided to leave RdKafkaException as is for now.

mhowlett · 2016-11-11T18:15:22Z

maybe the exception exposed to the user is never directly RdKafkaException, but maybe the exception that is has this as a child (in which case we still want it).

likewise with LibRdKafka, the Rd is appropriate i think as it relates to librdkafka.

ewencp · 2016-11-11T18:17:25Z

@mhowlett The main distinction in the code seems to be that stuff directly related to librdkafka uses LibRdKafka whereas things that are for the rdkafka-dotnet library use an unprefixed RdKafka. So RdKafkaException is the public interface users of rdkafka-dotnet care about. Elsewhere we've tried to simplify the naming to not be specific to librdkafka (which we consider an implementation detail, though a very excellent implementation detail). e.g., see KafkaException in the python client https://github.com/confluentinc/confluent-kafka-python/blob/master/confluent_kafka/src/confluent_kafka.c#L1506

mhowlett · 2016-11-11T18:25:05Z

@ewencp I think we need a KafkaException for sure (in fact i originally renamed RdKafkaException to that but changed it back). More specifically Confluent.Kafka.KafkaException. There is also a place for Confluent.Kafka.LibRdKafka.RdKafkaException (or something) i think. I was avoided making decisions around this in this PR - I think that is big enough for it's own PR as it involves carefully reviewing all usages of exceptions thoughout rdkafka-net.

mhowlett · 2016-11-11T18:26:56Z

And yes, RdKafkaException shouldn't be part of the public interface... (except as a child of KafkaException - analogous things elsewhere in .NET libraries).

edenhill · 2016-11-11T18:12:21Z

Confluent.Kafka.sln

 VisualStudioVersion = 14.0.25420.1
 MinimumVisualStudioVersion = 10.0.40219.1
-Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "RdKafka", "src\RdKafka\RdKafka.xproj", "{B2DDB635-4423-45D7-B3DC-F701E6010868}"
+Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Confluent.Kafka", "src\Confluent.Kafka\Confluent.Kafka.xproj", "{B2DDB635-4423-45D7-B3DC-F701E6010868}"


Should we have a new project uuid generated?

(I have no idea if it has any significance, but it looks random enough to be assumed as unique)

possibly. But I've done this sort of rename before a lot and never had problems and I'm also pretty sure that if you rename a project within VS it doesn't change the guid. I haven't been using VS yet (this was just a text editor change).

I wasn't worried about a plain renaming so much as if someone tries to open both rdkafka-dotnet and confluent-kafka-dotnet in the same IDE. Will they be treated as unique projects?

there would only potentially be problems if someone tried to add both projects to the same solution (they should never want to - maybe they want to link against the two libraries, but that is different). In the event they do, I just checked and VS automatically generates new guids, it doesn't let them conflict.

edenhill · 2016-11-11T18:13:36Z

README.md

+**confluent-kafka-dotnet** is a C# client for [Apache Kafka](http://kafka.apache.org/) based on [librdkafka](https://github.com/edenhill/librdkafka).

-Copyright (c) 2015-2016, [Andreas Heider](mailto:[email protected])
+Forked from [rdkafka-net](https://github.com/ah-/rdkafka-net)


..-dotnet

Also mention Andreas ("by Andreas Heider").

good points.

edenhill · 2016-11-11T18:29:10Z

README.md


 ## Examples

-### Producing messages


I guess you are removing this due to later API changes, but maybe keep a FIXME so we wont forget adding examples back in.

I guarantee you I won't forget to add examples back in... in fact, more likely to be reminded to do so by having nothing than something (that's wrong).

edenhill · 2016-11-11T18:29:54Z

doc/index.md

+=================================================

-**rdkafka-dotnet** is a C# client for [Apache Kafka](http://kafka.apache.org/) based on [librdkafka](https://github.com/edenhill/librdkafka).
+**confluent-kafka-dotnet** is a C# client for [Apache Kafka](http://kafka.apache.org/) based on [librdkafka](https://github.com/edenhill/librdkafka).


Some updates are C# -> .NET. Here C# is left unchanged.
What should be used?

good point. We should use .NET i think. (also changed in the project file).

edenhill · 2016-11-11T18:32:20Z

src/Confluent.Kafka/ErrorCode.cs

        /// <summary>Queue is full</summary>
        _QUEUE_FULL = -184,
        /// <summary>ISR count &lt; required.acks</summary>
-            _ISR_INSUFF = -183,


Are these indentation changes because you are using another editor?
Which one are you using?
We should probably have it match VS 2015 behaviour.

the original is screwed up in my VS with the standard tab size of 4 (I just booted it up to check, but I remember this from before). With the changes i made it displays correctly both in VS with tab spacing of 4 and my VS code where I set the tab size to 2. Based on that I don't know where his file would display correctly.

edenhill · 2016-11-11T18:33:55Z

src/Confluent.Kafka/ErrorCode.cs

+    /// <summary>
+    ///     Internal errors to rdkafka are prefixed with _
+    /// </summary>
+    public enum ErrorCode


Would it be possible to expose these programmatically?
This is an ever-increasing list and librdkafka already provides APIs to extract constant names, description, and value.

This is what I'm doing in the Go client:
https://github.com/confluentinc/confluent-kafka-go/blob/master/kafka/go_rdkafka_generr/go_rdkafka_generr.go

Python:
https://github.com/confluentinc/confluent-kafka-python/blob/master/confluent_kafka/src/confluent_kafka.c#L1384

it's possible to create an ErrorCode enum type at runtime from string/int values obtained from librdkafka. this might be something we want to do if we never want to expose the type to the user of the library (we want to though, right?). C# has 'dynamic' variables (duck typing) which means you can actually statically reference the dynamically generated enum values, but you loose IDE integration of course (and I think it's not really nice to include dynamic vars as part of the API, and no one else does).

we could also auto generate the enum by parsing librdkafka source ... again not something we want to do I think.

edenhill · 2016-11-11T18:35:10Z

src/Confluent.Kafka/Internal/LibRdKafka.cs

            internal static extern void rd_kafka_set_log_level(IntPtr rk, IntPtr level);

            [DllImport(DllName, CallingConvention = CallingConvention.Cdecl)]
            internal static extern IntPtr rd_kafka_outq_len(IntPtr rk);


Is there any way to use the librdkafka rdkafka.h header file instead of duplicating the prototypes here?
Even though the API is stable this does seem a bit error prone.

Not to my knowledge. I've always seen it done like this.

ewencp · 2016-11-11T18:21:28Z

Confluent.Kafka.sln

 VisualStudioVersion = 14.0.25420.1
 MinimumVisualStudioVersion = 10.0.40219.1
-Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "RdKafka", "src\RdKafka\RdKafka.xproj", "{B2DDB635-4423-45D7-B3DC-F701E6010868}"
+Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Confluent.Kafka", "src\Confluent.Kafka\Confluent.Kafka.xproj", "{B2DDB635-4423-45D7-B3DC-F701E6010868}"


Do we need new project UUIDs? Not sure how these are used exactly, but I'd guess that if we keep them the same it could cause conflicts with an rdkafka-dotnet checkout.

see other comment

ewencp · 2016-11-11T18:23:05Z

README.md

 ## Usage

-Just reference the [RdKafka NuGet package](https://www.nuget.org/packages/RdKafka)
+Just reference the [conflent-kafka-dotnet NuGet package](https://www.nuget.org/packages/confluent-kafka-dotnet)


typo: conflent -> confluent

ewencp · 2016-11-11T18:25:08Z

README.md

-Just reference the [RdKafka NuGet package](https://www.nuget.org/packages/RdKafka)
+Just reference the [conflent-kafka-dotnet NuGet package](https://www.nuget.org/packages/confluent-kafka-dotnet)

 ## Examples


If we're stripping out the contents of these sections can we drop them entirely? We can always restore them later (and we should file a JIRA to track that if we need to follow up later once we've cleaned up APIs etc).

up to you on this one - i don't have an opinion either way.

ewencp

LGTM assuming we actually follow up on the rest of the renaming stuff.

RdKafka -> Confluent.Kafka

3e26854

edenhill reviewed Nov 11, 2016

View reviewed changes

ewencp reviewed Nov 11, 2016

View reviewed changes

review changes

af9d736

ewencp approved these changes Nov 11, 2016

View reviewed changes

mhowlett merged commit 114d3f9 into confluentinc:master Nov 11, 2016

mhowlett mentioned this pull request Nov 15, 2016

Producer refactor #4

Merged

This was referenced Nov 22, 2016

Producer Refactor Mk II #8

Merged

Consumer API Design #9

Closed

mhowlett mentioned this pull request Dec 2, 2016

Producer API Design #10

Closed

bschmidtbauer mentioned this pull request Nov 30, 2017

Messages lost / unordered / strange behaviour #381

Closed

7 tasks

koushikchitta mentioned this pull request Mar 7, 2018

Invalid offsets on successful delivery of message with 0.11.3 #453

Closed

7 tasks

Chris2018Kafka mentioned this pull request Aug 9, 2018

Kafka 0.11.5 Will not reconnect-> 3/3 brokers are down error #584

Closed

ravi-sontale mentioned this pull request Sep 13, 2018

Producer gets stuck in an infinite loop and never produces #603

Closed

5 tasks

ChrisZollinger mentioned this pull request Sep 19, 2018

Broker metadata does not include topic on single machine #627

Closed

6 tasks

chiru1205 mentioned this pull request Dec 11, 2018

Producer message delivery issues #700

Open

ChrisZollinger mentioned this pull request Jan 10, 2019

Failed to load the librdkafka native library on AWS lambda? #572

Open

7 tasks

nitinpi mentioned this pull request Jan 31, 2019

Unable to receive messages : Group coordinator not available #756

Closed

7 tasks

cmills2000 mentioned this pull request Mar 6, 2019

await producer.ProduceAsync() never return #796

Closed

7 tasks

This was referenced Jun 3, 2020

Producer Metadata Refresh on Disconnected Broker Takes 20 seconds to Fail #1305

Closed

Producer Only Connecting to some Brokers at Startup #1308

Closed

Brokers Not Randomly Selected for Metadata Refresh #1309

Closed

montiver0311 mentioned this pull request Sep 23, 2020

Kafka Consumer lag and batching #1414

Open

8 tasks

Serg675 mentioned this pull request Dec 22, 2020

External Access to Apache Kafka on OpenShift/OKD from the .NET client. Is it possible? #1485

Open

8 tasks

foxesknow mentioned this pull request Jun 1, 2022

Delays in partition EOF being sent to additional instances #1824

Open

ElectricVampire mentioned this pull request Aug 17, 2023

Getting "SASL authentication error" in loop when password got auto rotated for MSK cluster #2102

Open

mapr-devops pushed a commit to mapr/confluent-kafka-dotnet that referenced this pull request Nov 6, 2024

Use Google Tink format for DEKs (confluentinc#2)

29100e9

RdKafka -> Confluent.Kafka #2

RdKafka -> Confluent.Kafka #2

Uh oh!

Conversation

mhowlett commented Nov 11, 2016

Uh oh!

ewencp commented Nov 11, 2016

Uh oh!

mhowlett commented Nov 11, 2016

Uh oh!

mhowlett commented Nov 11, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ewencp commented Nov 11, 2016

Uh oh!

mhowlett commented Nov 11, 2016

Uh oh!

mhowlett commented Nov 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ewencp left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mhowlett commented Nov 11, 2016 •

edited

Loading