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

Project backups #3852

Merged
merged 22 commits into from
Dec 20, 2021
Merged

Project backups #3852

merged 22 commits into from
Dec 20, 2021

Conversation

azhavoro
Copy link
Contributor

@azhavoro azhavoro commented Nov 2, 2021

image
image

Motivation and context

How has this been tested?

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

@azhavoro azhavoro changed the title [WIP] Project backups Project backups Nov 11, 2021
@nmanovic
Copy link
Contributor

@azhavoro , could you please add CHANGELOG? Please add some screenshots into the description of the PR as well, summarize changes which were done. It will help me in the future to prepare the next release and also understand changes not from code but from the description.

@nmanovic
Copy link
Contributor

@azhavoro , it looks really good. I found two problems:

  • When I "create from backup" for a project, the project isn't visible till I refresh the page
  • Preview for tasks and projects isn't the same (a different picture is used by a reason).

@azhavoro
Copy link
Contributor Author

@azhavoro , it looks really good. I found two problems:

  • Preview for tasks and projects isn't the same (a different picture is used by a reason).

@nmanovic Could you please attach a screenshot of this problem?

@nmanovic
Copy link
Contributor

@azhavoro , it looks really good. I found two problems:

  • Preview for tasks and projects isn't the same (a different picture is used by a reason).

@nmanovic Could you please attach a screenshot of this problem?

I cannot reproduce the issue now.

@nmanovic
Copy link
Contributor

One more problem. When I backup the project for the first time, I get the following error:

project_first_backup_lead_to_error

If I press the backup project again, the operation is successful.

@ActiveChooN
Copy link
Contributor

ActiveChooN commented Dec 3, 2021

"AttributeError: spec_id '156' is invalid\n"
Got this error, when trying to import the project.

@ActiveChooN
Copy link
Contributor

"AttributeError: spec_id '156' is invalid\n" Got this error, when trying to import the project.

Could not reproduce with new projects

cvat-core/src/project.js Outdated Show resolved Hide resolved
cvat-core/src/project.js Show resolved Hide resolved
cvat-core/src/project.js Show resolved Hide resolved
cvat-ui/src/reducers/notifications-reducer.ts Outdated Show resolved Hide resolved
cvat-ui/src/reducers/notifications-reducer.ts Outdated Show resolved Hide resolved
cvat-ui/src/reducers/projects-reducer.ts Outdated Show resolved Hide resolved
@bsekachev
Copy link
Member

bsekachev commented Dec 9, 2021

Some elements are not aligned as expected (my resolution is 3440x1440):

Screenshot_23

Need to add space between loading icon and text
Screenshot_24

Some attributes are not restored. See example below:
Original task
image

Exported annotations
image

Restored task (true is a default value)
image

@bsekachev
Copy link
Member

Subset is not backed up to the file. Always: "subset":""

As far as I understand we do not backup these fields:
image
Is it expected? Just in restored project/task there is no way to setup them.

@bsekachev
Copy link
Member

I created a project task with these parameters:
Screenshot_29

And can't dump the project anymore:
image

Created task is:
image

Jobs can be opened. I can share the file if you need.

bsekachev
bsekachev previously approved these changes Dec 17, 2021
@nmanovic
Copy link
Contributor

@azhavoro , could you please resolve conflicts?

@@ -21,7 +21,7 @@ class TokenAuthentication(_TokenAuthentication):
def authenticate(self, request):
auth = super().authenticate(request)
session = getattr(request, 'session')
if auth is not None and session.session_key is None:
if auth is not None and (session.session_key is None or (not session.modified and not session.load())):
Copy link
Contributor

Choose a reason for hiding this comment

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

@azhavoro , could you please put here some comments to understand these changes.

return backup.export(db_project, request)

@action(detail=False, methods=['POST'])
def backup(self, request, pk=None):
Copy link
Contributor

@nmanovic nmanovic Dec 20, 2021

Choose a reason for hiding this comment

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

@azhavoro , It looks like instead of POST /api/v1/tasks/backup we should have POST /api/v1/tasks with a corresponding data type. Also I don't like that for projects we create an empty project and after that import a dataset and for backup with have another approach. For creating tasks we have also two steps: create a tasks and send data after that.

Let's keep it as is for now and try to find a common solution later
@Marishka17 , please look at the inconsistency. It is something which we need to solve during REST API redesign.

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.

LGTM

@nmanovic nmanovic merged commit 32d9fb0 into develop Dec 20, 2021
@nmanovic nmanovic deleted the az/project_backup branch December 20, 2021 14:24
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.

4 participants