-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Updating Service-Bus dependency long from 4 to 5.2 #21208
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
API change check for API changes are not detected in this pull request for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thanks for the upgrade!
Can you please resolve the conflicts and merge this, Matt? |
There is an issue with the latest version of long, and it made the service-bus SDK unusable... |
@abouroubi have you tried with npm init -y
tsc --init
npm i long @azure/service-bus and found that |
Hello @jeremymeng,
Here is my code: import {
ProcessErrorArgs,
ServiceBusClient,
ServiceBusMessage,
ServiceBusReceivedMessage,
ServiceBusReceiver,
ServiceBusSender,
} from '@azure/service-bus';
import Long from 'long';
import { ConfigurationService } from '~shared/configuration/configuration.service';
export class QueueService {
private sbClient: ServiceBusClient;
private sender: ServiceBusSender;
private receiver: ServiceBusReceiver;
constructor(appConfig: ConfigurationService) {
this.sbClient = new ServiceBusClient(appConfig.ServiceBus.ConnectionString);
this.sender = this.sbClient.createSender(appConfig.ServiceBus.QueueName);
this.receiver = this.sbClient.createReceiver(appConfig.ServiceBus.QueueName);
}
async sendMessage(body: any, scheduledEnqueueTimeUtc: Date): Promise<string> {
const message: ServiceBusMessage = {
body: body,
};
const results = await this.sender.scheduleMessages(message, scheduledEnqueueTimeUtc);
return results[0].toString();
}
async cancelMessage(messageId: string) {
this.sender.cancelScheduledMessages(Long.fromString(messageId));
}
async receiveMessages(
processMessage: (message: { id: string; content: string }) => Promise<void>,
processError: () => Promise<void>,
) {
const process = async (brokeredMessage: ServiceBusReceivedMessage) => {
await processMessage(brokeredMessage.body);
await this.receiver.completeMessage(brokeredMessage);
};
const error = async (args: ProcessErrorArgs) => {
await processError();
};
this.receiver.subscribe(
{
processMessage: process,
processError: error,
},
{
autoCompleteMessages: false,
},
);
}
} |
@abouroubi Thanks for sharing the code! Please help me understand more since I don't see where the |
Yes the |
@abouroubi I tried your code with slight modification to get secret from environment variables. I get an id from const service = new QueueService();
const d = new Date();
const id = await service.sendMessage("hello-" + d.toUTCString(), new Date(d.getTime() + 60 * 1000));
console.log("Immediately canceling the message ", id);
await service.cancelMessage(id); also this merged PR is probably not the best place to continue discussion. Could you please open a new issue and share your minimum repro project? Thank you very much! |
Packages impacted by this PR
Issues associated with this PR
Describe the problem that is addressed by this PR
This updates the long npm package from 4.0 to 5.2.
What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?
This PR is to update the long dependency and fix any compilation errors which were found. In this case, two spec files needed an explicit import of the
long
module.Are there test cases added in this PR? (If not, why?)
Not applicable as this is an update to existing spec files for a dependency.
Provide a list of related PRs (if any)
Command used to generate this PR:**(Applicable only to SDK release request PRs)
Checklists