Skip to content

add client invocation for management sdk#2239

Open
ZSWY666 wants to merge 33 commits intoAzure:devfrom
ZSWY666:jiezong/clientInvecation
Open

add client invocation for management sdk#2239
ZSWY666 wants to merge 33 commits intoAzure:devfrom
ZSWY666:jiezong/clientInvecation

Conversation

@ZSWY666
Copy link
Copy Markdown
Contributor

@ZSWY666 ZSWY666 commented Dec 15, 2025

Add client invocation logic

@ZSWY666
Copy link
Copy Markdown
Contributor Author

ZSWY666 commented Dec 15, 2025

@ZSWY666 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree company="Microsoft"

@ZSWY666 ZSWY666 closed this Dec 15, 2025
@ZSWY666 ZSWY666 reopened this Dec 15, 2025
@ZSWY666 ZSWY666 closed this Dec 17, 2025
@ZSWY666 ZSWY666 reopened this Dec 17, 2025
@ZSWY666 ZSWY666 closed this Dec 25, 2025
@ZSWY666 ZSWY666 reopened this Dec 25, 2025
@ZSWY666
Copy link
Copy Markdown
Contributor Author

ZSWY666 commented Jan 9, 2026

PR should be blocked until the next release is done.

private HttpRequestMessage GenerateHttpRequest(string url, IDictionary<string, StringValues>? query, HttpMethod httpMethod, HubMessage? body, Type? typeHint)
{
var request = new HttpRequestMessage(httpMethod, GetUri(url, query));
request.Headers.Accept.Add(new System.Net.Http.Headers.MediaTypeWithQualityHeaderValue("application/octet-stream"));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is not the right place. BinaryPayloadContent already contains it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the BinaryPayloadContent, we just add a contentType instead of Accept, or maybe I lost something?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah I got the two mixed up. But it's still an appropriate place to put this in BinaryPayloadContent. RestClient is a general entry point, and we don't want JsonPayloadContent to receive binary response payload, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is a bit tricky here as this header is used to identify that "this request is come from ManagementSDK", so runtime will directly return raw bytes and let SDK to deserialize the result from client.
In this case, JsonPayloadContent will also receive the binary response payload

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why does JsonPayloadContent also receive the binary response payload?

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.

3 participants