Azure-sdk-for-js: [event-hubs] Refactor/cleanup for EventHubs now that the API has settled.

[richardpark-msft]

• [ ] Remove .receive() and associated tests
  The "firehose" of receiving events (all events, no specific ordering) has been superceded by
  receiveBatch() and the higher level subscribe methods.

  We can remove any tests related to receive as well as the receive method itself which will help our
  inner-loop and CI test times.

• [ ] Collapse/consolidate the object hierarchies for both EventHubConsumerClient and EventHubProducerClient.

  These higher level clients were built on other clients that were, at the time they were written, _also_ higher level clients. This has led to a bit of a deeper class hierarchy than was originally intended. We should see if there are sensible ways to collapse or eliminate some of the classes and simplify the internals a bit.

[richardpark-msft]

@chradek - per @ramya-rao-a 's request I'm making this more of a generic "refactors we should do now that we know what the actual surface API looks like". If you have more please add them.

[ramya-rao-a]

Notes from offline sync:

• We would like to remove the classes EventHubClient, EventHubProducer and EventHubConsumer.
• The EventHubProducerClient and EventProcessor should use EventHubSender and EventHubReceiver directly

[shabbyrobe]

Sorry if this is a bit OT, but will EventHubReceiver remain @internal or be removed? It seems like the "lower-level" library could still be useful in my situation, where I want to broadcast from latestEventPosition to a group of WebSockets and don't care about checkpointing - it's fine to just use the latest. I also considered passing in a "null" CheckpointStore but the docblock says "Users are not meant to implement an CheckpointStore" and there's only @azure/eventhubs-checkpointstore-blob so far. Is the advice about not implementing your own so you can reserve the right to adjust/remove/rework the API at a later date?

[chradek]

@shabbyrobe
If you instantiate EventHubConsumerClient without a checkpointstore, that should also give you the behavior you’re looking for. We included constructor overloads that don’t take a checkpointStore for those that want to do what you’re describing.

[shabbyrobe]

Ah thanks so much @chradek, that does look like what I'm looking for. I didn't see it at first when I clicked into the typings but it's there alright! As a further side note (apologies once again for being marginally OT), is this actually safe to do? To always just use latest and never store the checkpoint? Are there any caveats you're aware of?

[chradek]

@shabbyrobe With regards to if it's safe to not checkpoint, that really depends on your application's needs and whether it's ok for your app to potentially miss out on processing some events.

One thing that might be useful to know is that if you don't pass in a checkpointstore, you can still call updateCheckpoint(event) on the partitionContext passed into your processEvents handler. Then if the subscription from your subscribe call encounters an error where it needs to automatically restart processing events, it will continue from the last event (per partition) you passed to updateCheckpoint. The checkpoint in this case is saved in memory for that particular subscribe call so won't persist across multiple subscribe calls or if your node process restarts.

[ramya-rao-a]

Links for future reference:

PRs to update tests to use the public facing APIs in EventHubProducerClient and EventHubConsumerClient instead of the internal ones from EventHubClient:

• #9189
• #9191
• #9200
• #9341
• #9343
• #9427
• #9428
• #9431
• #9521

PRs to refactor the code

• #9520: Remove EventHubClient dependency in EventHubProducerClient
• #9548: Remove the middle layer of EventHubProducer
• #9600: Remove EventHubClient dependency in EventHubConsumerClient, EventProcessor, PumpManager and PartitionPump
• #9623: Remove the middle layer of EventHubConsumer

[ramya-rao-a]

Closing as all the above PRs have now been merged.