[Event Hubs] Define classes and interfaces for EPH as per API design #4416
Conversation
| private _partitionContext: PartitionContext; // for internal use by createCheckpoint | ||
| private _partitionManager: PartitionManager; // for internal use by createCheckpoint | ||
|
|
||
| get partitionContext(): PartitionContext { |
There was a problem hiding this comment.
partitionContext and partitionManager shouldn't be public for now. Customers will have access to both outside of CheckpointManager, and we want to the public API as simple as possible to start with.
There was a problem hiding this comment.
I added these public getters for now because build was failing with this error: _partitionContext' is declared but its value is never read.
I'll remove these getters later after the actual methods implementation.
There was a problem hiding this comment.
Ah gotcha. You could also just comment them out until we need them too :-)
| * The PartitionProcessorFactory is called by EPH whenever a new partition is about to be processed. | ||
| */ | ||
| export interface PartitionProcessorFactory { | ||
| createPartitionProcessor( |
There was a problem hiding this comment.
For JavaScript, it's more idiomatic to provide a function as the factory than a class/object that has a factory method on it.
If you remove the function name createPartitionProcessor but leave the rest of the signature, then PartitionProcessorFactory will just describe a function instead.
So this should work:
const partitionProcessorFactory: PartitionProcessorFactory = (
context: PartitionContext,
checkpointManager: CheckpointManager
) => {
return new PartitionProcessor();
};This was discussed in the API proposal but we had the existing example to make it easier for our C# friends to read :)
| export interface PartitionProcessorFactory { | ||
| createPartitionProcessor( | ||
| context: PartitionContext, | ||
| checkpointManger: CheckpointManager |
There was a problem hiding this comment.
Minor: checkpointManger should be checkpointManager
| * PartitionContext is passed into an EventProrcessor's initialization handler and contains information | ||
| * about the partition, the EventProcessor will be processing events from. | ||
| */ | ||
| export class PartitionContext { |
There was a problem hiding this comment.
@chradek Do we need this to be a class?
Or are we making it a class to ensure that the properties are readonly?
There was a problem hiding this comment.
It should be safe to make this an interface now. If we want to ensure the properties are readonly at runtime we can either freeze the object that implements the interface Object.freeze or create the object using Object.defineProperties
|
|
||
| public async createCheckpoint(offset: string, sequenceNumber: number): Promise<void>; | ||
|
|
||
| public async createCheckpoint( |
There was a problem hiding this comment.
This should be updateCheckpoint after the latest round of updates
As per feedback, we decided that EPH and Event Hubs client will ship in the same client library while the EPH plugins will each ship in their own separate client libraries.
This PR creates new classes and interfaces for Event processor host as per Proposed API design for EPH