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

59 ability to consume messages from superstream #149

Merged
merged 7 commits into from
Jan 12, 2024

Conversation

tarzacodes
Copy link
Contributor

No description provided.

@tarzacodes tarzacodes self-assigned this Jan 12, 2024
@tarzacodes tarzacodes linked an issue Jan 12, 2024 that may be closed by this pull request
@tarzacodes tarzacodes force-pushed the 59-ability-to-consume-messages-from-superstream branch from 13a80af to 577a2e1 Compare January 12, 2024 13:33
Copy link
Member

@icappello icappello left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@tarzacodes tarzacodes merged commit 5f1ab9d into main Jan 12, 2024
1 check passed
@tarzacodes tarzacodes deleted the 59-ability-to-consume-messages-from-superstream branch January 12, 2024 15:41
@@ -310,6 +321,11 @@ export class Client {
return res.ok
}

public async declareSuperStreamConsumer(superStream: string, handle: ConsumerFunc): Promise<SuperStreamConsumer> {
const partitions = await this.queryPartitions({ superStream })
return SuperStreamConsumer.create(handle, { locator: this, consumerRef: "test", partitions })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the consumerRef is hardcoded as "test", is it right?

@@ -82,6 +82,7 @@ export class StreamPublisher implements Publisher {
private scheduled: NodeJS.Immediate | null
private logger: Logger
private maxChunkLength: number
private isClosed = false
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly do not like "is" properties, I would suggest using only closed, wdyt?

export class SuperStreamConsumer {
private consumers: Map<string, Consumer> = new Map<string, Consumer>()
public consumerRef: string
private locator: Client
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is locator a good name for the client underneath the consumer?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the locator, that's the term we are using for the client that manages the connections right?

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

Successfully merging this pull request may close these issues.

Ability to consume messages from superstream
3 participants