[web-pubsub] Migrate @azure/web-pubsub to new core pipeline#16010
[web-pubsub] Migrate @azure/web-pubsub to new core pipeline#16010xirzec merged 22 commits intoAzure:mainfrom
@azure/web-pubsub to new core pipeline#16010Conversation
xirzec
added
Client
xirzec
requested review from
bterlson,
chradek,
jeremymeng,
ramya-rao-a and
witemple-msft
as code owners
|
|
||
| it("can manage users", async () => { | ||
| // `removeUserFromAllGroups` seems to take a very long time? | ||
| it.skip("can manage users", async () => { |
There was a problem hiding this comment.
I'm debugging this one, though it seems like the operation is always timing out on my test account
| async sendRequest(request: PipelineRequest, next: SendRequest): Promise<PipelineResponse> { | ||
| request.headers.set("Accept-Encoding", "gzip,deflate"); | ||
| // HEAD requests have no body | ||
| if (request.method !== "HEAD") { |
There was a problem hiding this comment.
Are there other places where a similar check could be needed? I just looked and I am not sure tbh.
There was a problem hiding this comment.
I looked at all the other supported HTTP methods and none of them had this semantic (even though ones that don't always return a body may in some cases)
| hasConnection(connectionId: string, options?: HasConnectionOptions): Promise<boolean>; | ||
| hasGroup(groupName: string, options?: HubHasGroupOptions): Promise<boolean>; | ||
| hasPermission(connectionId: string, permission: Permission, options?: HubHasPermissionOptions): Promise<RestResponse>; | ||
| hasPermission(connectionId: string, permission: Permission, options?: HubHasPermissionOptions): Promise<void>; |
There was a problem hiding this comment.
Seems an existing bug... hasPermission should return Promise<boolean>
| const binaryMessage = new Uint8Array(10); | ||
| res = await client.sendToAll(binaryMessage.buffer); | ||
| assert.equal(res._response.status, 202); | ||
| await client.sendToAll(binaryMessage.buffer, { onResponse }); |
There was a problem hiding this comment.
Also update the README.md to contain a sample usage for onResponse?
|
@vicancy I'm going to go ahead and merge this so the core fixes get into the next core release, but I'm happy to make additional web-pubsub changes/fixes! |
3 participants
Using the updated AutoRest code generator, I have migrated the package to use CoreV2.
Along the way I figured out we had some minor promise chaining issues in
core-rest-pipelineand also that Web PubSub will incorrectly returnContent-Encoding: gzipfor HEAD requests (despite having no response body.)