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

Large files uploads #3692

Merged
merged 52 commits into from
Dec 10, 2021
Merged

Large files uploads #3692

merged 52 commits into from
Dec 10, 2021

Conversation

klakhov
Copy link
Contributor

@klakhov klakhov commented Sep 20, 2021

Motivation and context

Resolved #956
Resolved #2332
Resolved #1616
Fixed large files uploading via webpage, implemented using chunk file uploads with upload progress bar.
Peek 2021-09-21 11-55

How has this been tested?

Manually tested with large videos, archives, 20k images

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.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

@nmanovic nmanovic changed the title [WIP] Large files uploads Large files uploads Sep 21, 2021
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat/apps/engine/views.py Outdated Show resolved Hide resolved
cvat/apps/engine/views.py Outdated Show resolved Hide resolved
cvat/apps/engine/views.py Outdated Show resolved Hide resolved
@klakhov klakhov changed the title Large files uploads [WIP] Large files uploads Sep 23, 2021
@bsekachev bsekachev self-assigned this Sep 23, 2021
@nmanovic
Copy link
Contributor

cvat/apps/engine/views.py Outdated Show resolved Hide resolved
cvat/apps/engine/views.py Outdated Show resolved Hide resolved
@nmanovic
Copy link
Contributor

@klakhov , I see that you have fixed only uploading data for tasks. I'm fine if you want to split the PR on several parts and implement huge file upload for other endpoints later.

cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat-core/src/server-proxy.js Outdated Show resolved Hide resolved
cvat/apps/engine/mixins.py Show resolved Hide resolved
cvat/apps/engine/mixins.py Show resolved Hide resolved
cvat/apps/engine/mixins.py Show resolved Hide resolved
cvat/apps/engine/mixins.py Outdated Show resolved Hide resolved
cvat/apps/engine/mixins.py Show resolved Hide resolved
@nmanovic
Copy link
Contributor

nmanovic commented Dec 3, 2021

@klakhov ,

  • the known list of files can be a good heuristics. But in this case we cannot finish upload if by a reason it is not possible to send a file from the list.
  • We can also use special acceptance headers (e.g. Upload-Start and Upload-Finish) like Tus does. It will be a part of our protocol. To support the legacy behaviors we can omit both headers and in the case the single request will be start and end at the same time.

What do you think?

@klakhov
Copy link
Contributor Author

klakhov commented Dec 3, 2021

@nmanovic
I'm fine with that. Lets use headers.

@klakhov
Copy link
Contributor Author

klakhov commented Dec 6, 2021

@nmanovic Could you please take a look on updated pr. I've removed /append, /tus, /finished endpoints and used Upload-Start, Upload-Finish, Upload-Multiple headers to indicate request purpose

cvat/settings/base.py Outdated Show resolved Hide resolved
@nmanovic
Copy link
Contributor

nmanovic commented Dec 9, 2021

@klakhov , could you please resolve conflicts?

Copy link
Contributor

@nmanovic nmanovic left a comment

Choose a reason for hiding this comment

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

Great job!

Comments for the future:

  1. I saw that in case of bulk upload we have the following operation for each file in python code: open, read, write, close. Probably we can just move them from a temporary location to upload directory. Probably it will be an optimization in the future.
  2. Need to update CLI to support the new protocol
  3. Need to add the protocol for all other upload operations (import task, import dataset, import project).

@nmanovic nmanovic merged commit cc057a7 into develop Dec 10, 2021
@nmanovic nmanovic deleted the kl/large-file-uploads branch December 10, 2021 15:11
@nmanovic nmanovic mentioned this pull request Mar 4, 2022
7 tasks
@nmanovic nmanovic mentioned this pull request Mar 15, 2022
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants