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

Missing APIs #139

Open
sir4ur0n opened this issue Apr 22, 2020 · 2 comments
Open

Missing APIs #139

sir4ur0n opened this issue Apr 22, 2020 · 2 comments

Comments

@sir4ur0n
Copy link
Contributor

Hello,

While working on a PR (I will open soon) to add documentation on many types and functions of hw-kafka-client, I have noticed several times some weird things while reading this library API on Hackage: some types / type classes are used in the public API but not exposed.

E.g. HasKafkaConf is not exposed by the library, yet it appears in various exposed functions, like topicOffsetsForTime :: (MonadIO m, HasKafka k) => k -> Timeout -> Millis -> TopicName -> m (Either KafkaError [TopicPartition]).

In turn, this means for API consumers that it's pretty unclear what must be passed to functions like topicOffsetsForTime.

List of missing API I found:

  • HasKafkaConf
  • HasKafka
  • HasTopicConf
  • KafkaConf
  • Kafka
  • TopicConf

Side question (maybe unrelated, let me know if this deserves another issue): Why are some functions using a concrete type (e.g. rebalanceCallback uses the concrete type KafkaConf) while others use the type class (e.g. statsCallback uses the constraint HasKafkaConf)?

@AlexeyRaga
Copy link
Member

These are basically classes that are not meant to be used directly by library users and are not meant to be given any new instances. The Has* classes only have instance for Consumer and Producer. The idea (an old one, a good or a bad one I don't know) was not to expose them to the client.
Library users should not need to know about KafkaConf, etc., some of these structures are even dangerous to expose since they hide unmanaged stuff...

Perhaps it can be done better, like exposing an umbrella typeclass like

class (HasKafka k, HasKafkaConf k) => KafkaClient k

and then exposing it for documentation reasons but not its superclasses...

@sir4ur0n
Copy link
Contributor Author

I think it's a good idea not to expose these structures and constraints, as they can be confusing and add unnecessary overhead for users (learning 3 new type classes).

Your solution sounds reasonable indeed, I wonder how that would be displayed in Haddock? I guess the constraint HasKafka k, HasKafkaConf k would be displayed but not linked (as these constraints would not be public), however the typeclass KafkaClient and the instances of KafkaClient for KafkaConsumer and KafkaProducer would be visible.

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

No branches or pull requests

2 participants