[web-pubsub] Migrate @azure/web-pubsub to new core pipeline
#16010
Conversation
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
| @@ -17,7 +17,10 @@ export function decompressResponsePolicy(): PipelinePolicy { | |||
| return { | |||
| name: decompressResponsePolicyName, | |||
| 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)
| group(groupName: string): WebPubSubGroup; | ||
| 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! |
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.)