Skip to content
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

Smart text serialization #12149

Merged
merged 2 commits into from
Jun 23, 2020
Merged

Smart text serialization #12149

merged 2 commits into from
Jun 23, 2020

Conversation

lmazuel
Copy link
Member

@lmazuel lmazuel commented Jun 22, 2020

Fix #12137

@lmazuel lmazuel requested a review from johanste June 22, 2020 18:31
@lmazuel lmazuel requested a review from xiangyan99 as a code owner June 22, 2020 18:31
xiangyan99
xiangyan99 previously approved these changes Jun 22, 2020
# A string is valid JSON, make the difference between text
# and a plain JSON string.
# Content-Type is a good indicator of intent from user
elif content_type and content_type.startswith("text/"):
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if we've created a bit of a mishmash in how we decide how to serialize/pass request bodies. We are using the Content-Type header, which parameters we pass in (form_data or content/stream_content) as well as looking at the content mixed together. For example, we will try to serialize the data as json even if I specify a completely different header (assuming I didn't pass in an Element instance that is :)). This is a reasonable first step given where we are.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'm not really happy by the black magic, it's why I took the simplest approach to avoid two much black magic: when you cannot guess at all if this is a string test, or a string token of JSON.

@lmazuel lmazuel merged commit 11c8fc6 into master Jun 23, 2020
@lmazuel lmazuel deleted the bug_12137 branch June 23, 2020 23:01
iscai-msft added a commit to iscai-msft/azure-sdk-for-python that referenced this pull request Jun 30, 2020
…into regenerate_keys

* 'master' of https://github.com/Azure/azure-sdk-for-python:
  don't use mgmt track2 (Azure#12183)
  Fix pip version requirement in build-test.yml (Azure#12148)
  Revert "Resolve Pip Related Errors (Azure#12157)", Pin VirtualEnv (Azure#12169)
  Smart text serialization (Azure#12149)
  Remove OSName Variable (Azure#12147)
  [formrecognizer] add strongly-typed receipt wrapper sample (Azure#12128)
  Add missing __init__ type annotations (Azure#12146)
  fix sample in readme (Azure#12144)
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.

Le Azure Core is broken when trying to send requests with content-type: text/plain.
3 participants