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

cvat-sdk: remove broken and unused body-to-params conversion #5240

Closed
wants to merge 3 commits into from

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Nov 4, 2022

It's broken, because it generates a multipart/form-data body that Django REST Framework cannot parse. DRF doesn't examine the individual Content-Type headers of parts, so it interprets the JSON-encoded values as str()-encoded values, which corrupts them.

Since this logic is also currently unused (and it's unclear whether it will ever be used again), remove it.

Motivation and context

This is a follow-up to #5058.

How has this been tested?

Relying on CI for this.

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.

It's broken, because it generates a multipart/form-data body that Django
REST Framework cannot parse.  DRF doesn't examine the individual
Content-Type headers of parts, so it interprets the JSON-encoded values as
`str()`-encoded values, which corrupts them.

Since this logic is also currently unused (and it's unclear whether it will
ever be used again), remove it.
@nmanovic
Copy link
Contributor

nmanovic commented Nov 7, 2022

/check

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

❌ Some checks failed
📄 See logs here

@SpecLad SpecLad marked this pull request as draft November 7, 2022 06:44
@nmanovic
Copy link
Contributor

@SpecLad , what is the next step with the PR?

@SpecLad
Copy link
Contributor Author

SpecLad commented Nov 21, 2022

@SpecLad , what is the next step with the PR?

It's on the backburner for now, because some tests don't pass and I can't fix them easily. It's not blocking anything, so I'll come bsck to it if this issue becomes relevant again (or if I have a spare minute).

@SpecLad
Copy link
Contributor Author

SpecLad commented Dec 16, 2022

I decided to fix this in a different way. I'll open a new PR, since it won't have much in common with this.

@SpecLad SpecLad closed this Dec 16, 2022
@SpecLad SpecLad deleted the dont-convert-body-to-params branch March 16, 2023 18:00
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.

2 participants