API change proposal

[vitold]

As a part of a new project, we are considering to use kanadi as a driver for streaming from nakadi.
Going through the source code I found some inconvenient parts which put some low-level responsibility on the client:
1)

 val callback = EventCallback.successAlways[SomeEvent] { eventCallbackData =>
    eventCallbackData.subscriptionEvent.events.getOrElse(List.empty).foreach{ 
      case e: Event.Business[SomeEvent] =>
        ..
      case e: Event.DataChange[_] =>
        ...
      case e: Event.Undefined[_] =>
        ...
    }
  }

I suggest to refactor it to:

val callback = EventCallback.successAlways[Event.Business[SomeEvent]] { // Or other message type
  events => // which alredy has type List[Event.Business[SomeEvent]]
    ...
  }
}

The reason for this is the api of nakadi do not allow you to have several message types in the same event type, so this pattern matching becomes an unnecessary boilerplate every time when you creating the event batch handler.

2)
Change the type of SubscriptionEvent.events from Option[List[Event[T]]] to NonEmptyList[Event[T]]
As a user of the driver, you do not expect to receive an empty batch. And in case of an error, it should be reported internally in the driver.

[i-am-the-slime]

I realised yesterday that it is possible to have subscriptions for multiple event types. I am not sure that this allows for different kinds of events. Maybe it makes sense to keep the old API and just add convenience methods on top for the 99% case of always the same event type.

[vitold]

@i-am-the-slime Api extension even better. For backward compatibility for example.

[mdedetrich]

Sorry for the late response guys, I missed my github notifications

I am definitely up for making a release of Kanadi that fixes the API, there are quite a few historical mistakes. @i-am-the-slime has a point though, you can technically subscribe to multiple event types which have different structures (i.e. undefined vs data vs business)

The only issue is that I am currently maintaining 3 branches (yay for binary compatibility!) so I need to think about how to approach this.

[mdedetrich]

@vitold But yeah, a PR to add an extra method for this case is always welcome, that way its backwards compatible!