Skip to content

Conversation

@antkmsft
Copy link
Member

@antkmsft antkmsft commented Jan 31, 2023

Please help me to find out the truth: either all SDK should implement conformance to these 2 scenarios, or it should not be allowed - so then the scenarios should be removed, and these SDKs who have logic to make it work probably need to undo the code that they have to enable it.

  • Java, .NET, and Python do allow to override User-Agent, C++ and Go do not;
  • .NET, Python, and Go do allow to override x-ms-client-request-id, C++ does not.

I am having several conversations with people saying that User-Agent and x-ms-client-request-id overriding should not be allowed:

@ahsonkhan
Copy link
Contributor

@mikekistler, @lmazuel could you help clarify what the consistent behavior should be?

@mikekistler
Copy link
Member

I think this is a question for the SDK architects to decide, but I can give my opinion.

  • User-Agent: I think we should allow users to override but also send x-ms-useragent with the original contents of user-agent so we can still get telemetry.
  • x-ms-client-request-id: I think we should let users specify this value. It is, after all, for "clients".

@jhendrixMSFT
Copy link
Member

The vast majority of these tests were written during track 1 and have not been updated to reflect our current thinking.

This test specifies user-agent in its OpenAPI spec as an input parameter which I believe is something we'd never allow in a production API. So already I think the test is suspect.

@lmazuel
Copy link
Member

lmazuel commented May 8, 2023

Customers should be allowed to override both, but not through Swagger, through Azure-Core features.

IOW, I would agree to remove the "User-Agent" parameter from the Swagger, as it should not be necessary to trigger an override. But the test itself checking that people can override the value should stay.

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.

5 participants