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

[Service Bus] isReceivingMessages on subscribe() output instead of receiver #9746

Closed
ramya-rao-a opened this issue Jun 26, 2020 · 4 comments
Closed
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. Service Bus
Milestone

Comments

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jun 26, 2020

This is the current shape of the API for the Receiver in v7 of Service Bus:

export interface Receiver<ReceivedMessageT> {
    close(): Promise<void>;
    entityPath: string;
    getMessageIterator(options?: GetMessageIteratorOptions): AsyncIterableIterator<ReceivedMessageT>;
    isClosed: boolean;
    isReceivingMessages(): boolean;
    peekMessages(options?: PeekMessagesOptions): Promise<ReceivedMessage[]>;
    receiveDeferredMessages(sequenceNumbers: Long | Long[], options?: OperationOptions): Promise<ReceivedMessageT[]>;
    receiveMessages(maxMessages: number, options?: ReceiveMessagesOptions): Promise<ReceivedMessageT[]>;
    receiveMode: "peekLock" | "receiveAndDelete";
    subscribe(handlers: MessageHandlers<ReceivedMessageT>, options?: SubscribeOptions): void;
}

The isReceivingMessages() is mainly useful for the subscribe() model and not so much for the others

  • It does not apply at all for peekMessages() and receiveDeferredMessages()
  • receiveMessages() doesnt really need this as one can check the status of the promise returned by it to know if the receiving is done or not
  • The iterator model does not need this as each loop can be awaited as well.

#9493 tracks the proposal to make subscribe() return a object that is closable.
This issue is to move the isReceivingMessages() into the above object.

Or at the very least, remove it from the Receiver and add it based on user feedback.

cc @richardpark-msft, @HarshaNalluru, @chradek

@ghost ghost added the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 26, 2020
@ramya-rao-a ramya-rao-a added Client This issue points to a problem in the data-plane of the library. Service Bus labels Jun 26, 2020
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 26, 2020
@ramya-rao-a ramya-rao-a added this to the [2020] July milestone Jun 26, 2020
@ramya-rao-a
Copy link
Contributor Author

Side note: In Event Hubs, we went with isRunning on the Subscription that was returned bu subscribe().
Not sure if that was meant for a purpose different than what isReceivingMessages() in Service Bus would serve

@chradek
Copy link
Contributor

chradek commented Jun 26, 2020

I think we went with isRunning to signify that the subscription wasn't closed, since technically the subscription might not be receiving events yet if it hasn't connected to any partitions. It's technically correct...

@richardpark-msft
Copy link
Member

I'd talked with @ramya-rao-a about this - currently leaning towards removing it since it sounds like we added it because our receiver was unreliable (and this was a workaround for customers to detect that). There have been numerous reliability fixes in track 2 so removing this flag feels within reach.

@HarshaNalluru
Copy link
Member

Done with #9875

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. Service Bus
Projects
None yet
Development

No branches or pull requests

4 participants