Big API changes (replacing PR "Parameterize Monad")

[mtesseract]

Foreword: This started as an attempt to simply modify the Config to be parameterized over some "base monad" b, effectively allowing the user to decide in which monads the provided callbacks are supposed to be run.

Since this change was not easy to be done in isolation, the whole type foundation of this package was reworked, now following a more MTL-style approach by defining monads MonadHttp, MonadHttpStream and MonadNakadi.

At the same time a goal was to decouble IO from this project as much as possible, allowing for easy IO-free mocking, for instance.

This required fundamental changes to the HttpBackend mechanism. The old mechanism was removed. Using custom HTTP backends is now to be done by implementing MonadNakadiHttp resp. MonadNakadiHttpStream for a custom monad.

A general hindrance for this API redesigning was the rather cumbersome API for event consumption. This is why, at the same time, these APIs were completely reworked, now exposing a much simpler interface to the user.

Since these building lots (type foundation, complex event consumption API, configuration API, IO decoupling and HttpBackend mocking) were all interconnected tightly, this PR now became bigger than anticipated.

Summary of the changes:

• Completely rewritten the type foundation for nakadi-client, employing a more MTL-style approach.

• Simplify newConfig API for creating new configurations so that it does not require to be run in a monad; remove newConfig'.

• Change Config type so that most entries are now Maybes inside of it.

• Provide type alias ConfigIO.

• If the HTTP manager is Nothing, use a global HTTP Manager (it is recommended to set a dedicated HTTP manager, though).

• Export new function: newConfigWithDedicatedManager

• Export new function: setHttpManager.

• Remove function: setHttpBackend. Changing the HttpBackend does now require implementing a new instance of MonadNakadiHttp. This was necessary due to the fact that the "standard" HTTP backend involving retries and streaming was heavily tight to IO.

• Remove *R suffixed functions.

• Change all nakadi-client service functions so that they need to be run in a MonadNakadi implementing monad, having a configuration available implicitly.

• Replace throwIO with throwM.

• Remove eventSource function, provide the functions eventsProcessConduit and eventsProcess instead, simplifying their APIs (getting rid of the extra ReaderT).

• Rename function eventPublish → eventsPublish.

• Simplify internal API of httpJsonBodyStream; it is not using ResourceT anymore, but a simple bracket pattern (requiring MonadMask).

• Add missing lenses to Lenses.hs

• Simplify StreamConnectCallback API (the user should not need to be passed his own log func).

• New Nakadi exception: StreamIdMissing.

• Generalize EventStreamBatch type so that the provided type parameter a directly denotes the type of the contained events — without wrapping in EventEnriched.

• New data type: DataChangeEventEnriched.

• Remove data type SubscriptionEventStreamContext.

• Remove overly complicated subscriptionSource API, provide subscriptionProcess and subscriptionProcessConduit instead.

• Rewritten test to use the new interface for replacing the standard HTTP backend.

• Generally, adjusted tests to the API changes.

• New example below tests/: A simple echo service.

• Added more tests.

@etorreborre Sorry it became that big.

[etorreborre]

Thanks for the summary, I will try to have a fresh look at this PR next week but more importantly do a branch of our app with those changes to see if everything works as expected in terms of API and runtime behaviour.

[mtesseract]

FYI, @pascalh.

[mtesseract]

I have just added a convenience layer for easy mocking of http backends. @etorreborre

[etorreborre]

Thanks a lot @mtesseract I'll try to give it a go today!

[mtesseract]

Update: After this design journey I was finally able to save the best of both worlds: (1) The easy mocking API based on the idea of storing an HttpBackend directly in the Config and (2) the decoupling of IO. For this I had to insert some more MonadMask constraints, but this is totally fine in my opinion.

The MonadNakadiHttp based solution prepared the rest of the code base for the essential distinction between IO and non-IO (for example, the class methods did not leak out any IO types anymore). For some reason I have not seen this solution before. :-( With this in place, this modification was rather short.

Thanks for your feedback / your criticism with respet to getting this right. I appreciate it! And I'm sorry if you wasted too much time on the MonadNakadiHttp thing,

[etorreborre]

No worries, it is great that we can finally end up on this solution, I will give it a go on Monday (I meetings permit...).

[etorreborre]

I am almost done on using this branch in sparrow, some tests are not passing but I think it's my fault. One request, is it possible to add an instance of MonadNakadi for WriterT m? (I'm currently using StateT m as a workaround).

[reactormonk]

You should be able to cut down on the boilerplate of writing mtl instances via https://lexi-lambda.github.io/blog/2017/04/28/lifts-for-free-making-mtl-typeclasses-derivable/

[mtesseract]

I have implemented and integrated the MonadNakadiBase monad — without incoherent instances, but with some well-defined overlapping semantics.

MonadNakadi b m now requires MonadNakadiBase b m. It encodes the required lifting functionality.

nakadi-client provides MonadNakadiBase instances for several instances, e.g. IO and ReaderT r IO. In case a user has some very specific monad transformer stack set as Nakadi base monad, that can also be used. Either by defining a MonadNakadiBase instance (no methods need to be implenented) or using the included runNakadiBaseT wrapper (does not require additional instances). There's a test that demonstrates this.

With this in place, it works out of the box to do something like runLoggingT . runNakadiT conf, something that didn't work previously.

Keeping MonadNakadi and MonadNakadiBase seperate simplifies the code significantly. For the most common use cases the user does not have to do anything, it just works.

I have also simplified MTL deriving somewhat.

[mtesseract]

@etorreborre I have force-pushed onto this branch in order to clean up the history. Feel free to review. If you have questions, please ask. :)

[etorreborre]

That looks really good to me. Can't wait to upgrade :-).

[mtesseract]

So, I take it, you are fine with merging? As I said, there is more to come. I would simply be very happy about being able to continue with much smaller PRs. :-)

[etorreborre]

Yes let's merge.