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

Fix TUS offset queries in production environments #5204

Merged
merged 3 commits into from
Nov 8, 2022

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Oct 31, 2022

Previously, mod_wsgi would convert HEAD requests into GET, which would be rejected, so clients were unable to resume an upload that failed midway through.

To make use of this, update the SDK code to enable upload resumption.

Motivation and context

Fixes #4839.

How has this been tested?

SDK unit tests. For the old server workaround, I manually disabled the fix and checked that the tests still pass.

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.

@SpecLad SpecLad marked this pull request as ready for review October 31, 2022 12:56
@zhiltsov-max
Copy link
Contributor

@SpecLad, could you add a test which resumes previous uploading?

@SpecLad
Copy link
Contributor Author

SpecLad commented Nov 4, 2022

@SpecLad, could you add a test which resumes previous uploading?

I wrote this test, but it needs the fix in #5235 to work.

@SpecLad
Copy link
Contributor Author

SpecLad commented Nov 7, 2022

Okay, test added.

tests/python/sdk/test_tasks.py Show resolved Hide resolved
@zhiltsov-max
Copy link
Contributor

Can be merged after the imports changed.

Previously, `mod_wsgi` would convert `HEAD` requests into `GET`, which
would be rejected, so clients were unable to resume an upload that failed
midway through.

To make use of this, update the SDK code to enable upload resumption.
@zhiltsov-max zhiltsov-max merged commit aadfd88 into cvat-ai:develop Nov 8, 2022
@SpecLad SpecLad deleted the enable-head branch November 8, 2022 17:32
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.

Can't fully utilize TUS protocol
3 participants