search

Azure / azure-event-hubs-node

Node client library for Azure Event Hubs https://azure.microsoft.com/services/event-hubs
MIT License
50 stars 46 forks source link

Coerce patitionKey into a string #180

Closed sabeegrewal closed 5 years ago

sabeegrewal commented 5 years ago

When users set a partitionKey, the library should convert the partitionKey to a type of string. All other libraries in the EventHubs ecosystem expect a string type.

Continuation of: Azure/azure-event-hubs-spark#424

ramya-rao-a commented 5 years ago

@sabeegrewal As per your comment in https://github.com/Azure/azure-event-hubs-spark/issues/424#issuecomment-454482170

In general, we expect the partitionKey to be a string.

By we, I am assuming you are referring to the Event Hubs service itself. Right?

Seems that the node.js library should coerce partitionKeys into string types.

What I am worried about here is breaking existing customers by making this change especially if they rely on the partitionKey being used as is. Can you elaborate on how exactly is partitionKey used by Event Hubs?

When the passed value is not a string, does Event Hubs ignore it? Now, if we start converting the given value to string, will that change the behavior regarding which partition the data gets sent to?

sabeegrewal commented 5 years ago

@sjkwak can speak to this more.

Yea, the service and all statically-typed clients treat partitionKey as a string.

ramya-rao-a commented 5 years ago

Thanks @sabeegrewal

We learnt that by converting user provided partitionKey to a string, a user who has been using a non string partition key will now have their messages sent to a totally new partition. This is a breaking change especially if the partition id provided when they are receiving messages are hard-coded.

We are leaning towards throwing an error message in such cases and make a major version update. This way the experience for existing customers won't change unless they opt in to a new major version.

ramya-rao-a commented 5 years ago

We have just released a new version 2.0.0 for the Event Hubs library which includes the change to throw an error if a message is being set with a partitionKey which is not a string.