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

Requests page #8095

Merged
merged 198 commits into from
Jul 4, 2024
Merged

Requests page #8095

merged 198 commits into from
Jul 4, 2024

Conversation

klakhov
Copy link
Contributor

@klakhov klakhov commented Jun 27, 2024

Motivation and context

This PR introduces new page with information about data processing(status and progress). For now it will support: task creation, import/export.

For previous discussions refer to: #7537

How has this been tested?

Checklist

  • I submit my changes into the develop branch
  • I have created a changelog fragment
  • I have updated the documentation accordingly
  • I have added tests to cover my changes
  • [ ] I have linked related issues (see GitHub docs)
  • I have increased versions of npm packages if it is necessary
    (cvat-canvas,
    cvat-core,
    cvat-data and
    cvat-ui)

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.

@zhiltsov-max
Copy link
Contributor

  1. The sorting header seems to be missing on the Requests tab. It's there on all the other tabs, but not in this one
    image
  2. I'd just show the downloading button on the card. I don't see the point in the actions section with just 1 button. Actions can include some additional actions, but downloading is the key purpose of these cards.
    image
  3. When we click export, the annotations are not downloaded automatically. To export, we need to make several extra clicks. If we export multiple tasks, this will be multiplied by the number of tasks exported. To me, it doesn't look a good decision. Probably, we can download automatically, if we know there were some downloading requests in this UI session (i.e. before the window was refreshed). If the window was reopened, the exports can be found in the list, as implemented.

@novda
Copy link
Member

novda commented Jun 28, 2024

I see that the latest test runs were successful, looks like an unstable rest-api test. Marked a failed test as unstable, if any test fails which is not related to the PR, please tag me

@bsekachev
Copy link
Member

@klakhov

Please, resolve conflicts

job = self._get_rq_job_by_id(pk)

if not job:
return HttpResponseNotFound(f"There is no request with specified id: {pk}")

Check warning

Code scanning / CodeQL

Reflected server-side cross-site scripting Medium

Cross-site scripting vulnerability due to a
user-provided value
.
field_name=StorageType.TARGET,
)
except ValueError as ex:
raise serializers.ValidationError(str(ex)) from ex

Check warning

Code scanning / CodeQL

Information exposure through an exception Medium

Stack trace information
flows to this location and may be exposed to an external user.
@klakhov
Copy link
Contributor Author

klakhov commented Jul 1, 2024

  1. The sorting header seems to be missing on the Requests tab. It's there on all the other tabs, but not in this one

The sorting/filtering functionality was not planned in this iteration

  1. I'd just show the downloading button on the card. I don't see the point in the actions section with just 1 button. Actions can include some additional actions, but downloading is the key purpose of these cards.

I will think about this. We may need to re-design the card a little bit. Probably this enhancement can be moved to second iteration.

  1. When we click export, the annotations are not downloaded automatically. To export, we need to make several extra clicks. If we export multiple tasks, this will be multiplied by the number of tasks exported. To me, it doesn't look a good decision. Probably, we can download automatically, if we know there were some downloading requests in this UI session (i.e. before the window was refreshed). If the window was reopened, the exports can be found in the list, as implemented.

I belive we discussed automatic downloading several times on different meetings, in the end we decided to stick with the simplier version - download by clicks(sometimes we may want to have control what we download and in what order). I agree that sometimes it can be not-so-useful. Probably there should be a setting controlling the behavior. Anyway, the feature looks like an enhancement that needs discussion.

@bsekachev
Copy link
Member

bsekachev commented Jul 2, 2024

I think Expires on on resources exported to a cloud storage may confuse users a little bit

image

Upd: Better to use Finished on in this case.

@bsekachev
Copy link
Member

As discussed, let's increase default value for DATASET_CACHE_TTL.
I believe we may use 24 hours instead of 10 hours and also, avoid dividing this value by 3 for projects

File: cvat/apps/dataset_manager/views.py, line 42

CC: @zhiltsov-max

@klakhov
Copy link
Contributor Author

klakhov commented Jul 2, 2024

I think Expires on on resources exported to a cloud storage may confuse users a little bit

image

Upd: Better to use Finished on in this case.

Applied

Copy link

sonarcloud bot commented Jul 2, 2024

@nmanovic nmanovic merged commit c9f1cff into develop Jul 4, 2024
33 checks passed
@nmanovic nmanovic deleted the kl/data-processing branch July 4, 2024 16:01
@novda novda mentioned this pull request Jul 10, 2024
7 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
Development

Successfully merging this pull request may close these issues.

None yet

7 participants