Git flexibility after task creation #3886

PMazarovich · 2021-11-09T11:18:11Z

Motivation and context

It is necessary sometimes to be able to change git repository data for the task not only during task creation.

With this changes it is possible to change LFS, format and repository URL from task page after task creation.

How has this been tested?

It was tested manually

Checklist

I submit my changes into the develop branch
I have added description of my changes into CHANGELOG file
I have updated the documentation accordingly
I have added tests to cover my changes
I have linked related issues (read 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.
I have updated the license header for each file (see an example below)

# Copyright (C) 2021 Intel Corporation
#
# SPDX-License-Identifier: MIT

bsekachev · 2021-11-12T09:40:04Z

cvat/apps/engine/views.py

+    @action(detail=True, methods=['POST'], serializer_class=None, url_path='updateGitData')
+    def update_git_repo(self, request, pk):
+        data = request.data
+        gitDataObj = drm.GitData.objects.filter(task_id=pk)[0]
+        if data.get("type") == "url":
+            gitDataObj.url = request.data.get('value')
+            gitDataObj.save(update_fields=["url"])
+        elif data.get("type") == "lfs":
+            gitDataObj.lfs = bool(request.data.get('value'))
+            gitDataObj.save(update_fields=["lfs"])
+        elif data.get("type") == "format":
+            gitDataObj.format = request.data.get('value')
+            gitDataObj.save(update_fields=["format"])
+        django_response = HttpResponse(
+            status=http.HTTPStatus.OK,
+        )
+        return django_response


This is PATCH request, not POST. We do not create any new resources.

It should not be an action for a task. I believe PATCH /api/v1/git/repository/<id> is a nice way. PATCH /api/v1/git/repository//patch is also applicable for dataset_repo app (according to its current API).

bsekachev · 2021-11-12T09:44:39Z

cvat/apps/dataset_repo/dataset_repo.py

+                slogger.task[self._tid].info("Local repository initialization..")
+                shutil.rmtree(self._cwd, True)
+                self._clone()
+                #raise git.exc.GitError("Actual and saved repository URLs aren't match")


Do you need this comment now?

bsekachev · 2021-11-12T09:48:07Z

cvat-core/src/api-implementation.js

+        cvat.server.changeRepo.implementation = async (taskId, type, value) => {
+            const result = await serverProxy.server.changeRepo(taskId, type, value)
+            return result
+        }
+


Please, try to avoid adding new methods to cvat-core. All the logic related with creating new repositories are in /cvat/cvat-ui/src/utils/git-utils.ts and, patching, I believe, should be the same place. You can add new redux actions if they are nesassary from your opinion

bsekachev · 2021-11-12T09:49:46Z

cvat-ui/src/components/task-page/details.tsx

+import { Typography } from 'antd';
+import { Select } from 'antd';
+import { Checkbox } from 'antd';
+const { Option } = Select;
+const { Paragraph } = Typography;


Please, consider:

import Paragraph from 'antd/lib/typography/Paragraph'; import Checkbox from 'antd/lib/checkbox'; import Select from 'antd/lib/select';

bsekachev · 2021-11-12T09:51:16Z

cvat-ui/src/components/task-page/details.tsx

@@ -32,6 +40,8 @@ interface Props {
    projectSubsets: string[];
    cancelAutoAnnotation(): void;
    onTaskUpdate: (taskInstance: any) => void;
+    dumpers: [];


Suggested change

dumpers: [];

dumpers: any[];

bsekachev · 2021-11-12T09:59:54Z

cvat-ui/src/components/task-page/details.tsx

        return (
-            !!repository && (
+            !!repository && user.isStaff && (


Why only staff can do it? Why not a task owner or smbd else?

bsekachev · 2021-11-12T10:00:48Z

cvat-ui/src/components/task-page/details.tsx

-                        </p>
+                        <Paragraph editable={{ onChange: this.onChangeRepoValue }}>{this.state.changedRepo}</Paragraph>
+                        <Text strong className='cvat-text-color'>Using format</Text>
+                        <Select onChange={this.onChangeFormatValue} style={{ width: '100%' }} initialValue={format} defaultValue={format}>


Please, put static styles to .scss files.

bsekachev · 2021-11-12T10:02:04Z

cvat-ui/src/containers/task-page/details.tsx

@@ -18,6 +18,8 @@ interface StateToProps {
    activeInference: ActiveInference | null;
    installedGit: boolean;
    projectSubsets: string[];
+    dumpers: [];


Suggested change

dumpers: [];

dumpers: any[];

bsekachev · 2021-11-12T10:02:30Z

cvat-ui/src/components/task-page/details.tsx

+
+    onChangeLFSValue = (value): void => {
+        const { taskInstance } = this.props
+        this.setState({lfs: value.target.checked});


Suggested change

this.setState({lfs: value.target.checked});

this.setState({ lfs: value.target.checked });

bsekachev · 2021-11-12T10:02:37Z

cvat-ui/src/components/task-page/details.tsx

+
+    onChangeFormatValue = (value: string): void => {
+        const { taskInstance } = this.props
+        this.setState({format:value});


Suggested change

this.setState({format:value});

this.setState({ format: value });

bsekachev · 2021-11-12T10:03:19Z

Hi, thank you for the contribution. Please, look at linter issues.

PMazarovich · 2021-11-26T15:31:13Z

@bsekachev , Hello, I'll try to fix these problems approx, from 6th December

PMazarovich · 2021-12-06T16:23:21Z

@bsekachev , hello, PR fixed as you have suggested.

bsekachev · 2022-01-10T09:38:03Z

cvat-ui/src/components/task-page/details.tsx

        return (
            !!repository && (
                <Row>
                    <Col className='cvat-dataset-repository-url'>
                        <Text strong className='cvat-text-color'>
                            Dataset Repository
                        </Text>
+                        <Paragraph editable={{ onChange: this.onChangeRepoValue }}>{changedRepo}</Paragraph>
+                        <Text strong className='cvat-text-color'>Using format</Text>
+                        <Select onChange={this.onChangeFormatValue} className='cvat-repository-format-select' initialValue={format} defaultValue={format}>


We should do this component managed this way:

Suggested change

<Select onChange={this.onChangeFormatValue} className='cvat-repository-format-select' initialValue={format} defaultValue={format}>

<Select onChange={this.onChangeFormatValue} className='cvat-repository-format-select' value={format}>

bsekachev · 2022-01-10T09:38:44Z

cvat-ui/src/components/task-page/details.tsx

@@ -102,7 +111,9 @@ export default class DetailsComponent extends React.PureComponent<Props, State>

                    this.setState({
                        repository: data.url,
+                        changedRepo: data.url,


Why do we need one more field to store changed repository?

bsekachev · 2022-01-10T09:46:39Z

cvat-ui/src/components/task-page/details.tsx

@@ -130,6 +141,26 @@ export default class DetailsComponent extends React.PureComponent<Props, State>
        this.mounted = false;
    }

+    private onChangeRepoValue = (value: string): void => {
+        const { taskInstance } = this.props;
+        this.setState({ changedRepo: value });


First we can get the current state value of repository state and keep it in memory.
Then do setState with the new repository value. At the same time I would add repositoryUpdating flag (to the state). When the flag is true, I would show a loading icon near the field, and disable all the repository fields (Because updating can take long time)
Finally we need to start server request.
In then/catch set appropriate state for repository and updatingRepository. For example in catch you can revert the previous repository value (request failed)

What do you think?

@bsekachev , the thing is, that according to current logic, there is no immediate initialization.
Initialization happens only after other events like get/push etc. changeRepo request triggers only changes of appropriate rows in DB. That is why there is no long-run jobs.

Sorry, I didn't catch your idea. There is no long-run server jobs, but there is a server request and its routing/handling can take time (quite long btw for highly loaded servers like cvat.org, for example), so while this processing a user should see an updated field I believe (just strange to see unedited value after a user finishes editing, do not you think?). And better if a user saw some loading status to understand that there changes weren't actually applied yet.

What is more, the request may be failed, in this case we should revert fiels (otherwise the user sees data which are not real, I mean they are not saved on the sever), it is why I suggested to handle with catch additionally.

bsekachev · 2022-01-10T09:46:56Z

cvat-ui/src/components/task-page/details.tsx

+    };
+
+    private onChangeLFSValue = (value): void => {
+        const { taskInstance } = this.props;


The same comment like above for other change callbacks

bsekachev · 2022-01-10T09:47:00Z

cvat-ui/src/components/task-page/details.tsx

+    };
+
+    private onChangeFormatValue = (value: string): void => {
+        const { taskInstance } = this.props;


The same comment like above for other change callbacks

bsekachev · 2022-01-10T09:48:41Z

Could you please merge develop, provide a screenshot to the PR body, and update CHANGELOG.md?

bsekachev · 2022-01-10T11:24:27Z

Current UI looks quite ugly, do not you think so?

I would propose some changes to do it like this:

My changes are

cvat-ui/src/components/task-page/details.tsx

private renderDatasetRepository(): JSX.Element | boolean {
        const { taskInstance, dumpers } = this.props;
        const {
            repository, repositoryStatus, format, lfs,
        } = this.state;
        return (
            !!repository && (
                <Row>
                    <Col className='cvat-dataset-repository-url'>
                        <Text strong className='cvat-text-color'>
                            Dataset Repository
                        </Text>
                        <Paragraph>
                            <Text editable={{ onChange: this.onChangeRepoValue }}>
                                {repository}
                            </Text>
                            {repositoryStatus === 'sync' && (
                                <Tag color='blue'>
                                    <CheckCircleOutlined />
                                    Synchronized
                                </Tag>
                            )}
                            {repositoryStatus === 'merged' && (
                                <Tag color='green'>
                                    <CheckCircleOutlined />
                                    Merged
                                </Tag>
                            )}
                            {repositoryStatus === 'syncing' && (
                                <Tag color='purple'>
                                    <LoadingOutlined />
                                    Syncing
                                </Tag>
                            )}
                            {repositoryStatus === '!sync' && (
                                <Tag
                                    color='red'
                                    onClick={(): void => {
                                        this.setState({
                                            repositoryStatus: 'syncing',
                                        });

                                        syncRepos(taskInstance.id)
                                            .then((): void => {
                                                if (this.mounted) {
                                                    this.setState({
                                                        repositoryStatus: 'sync',
                                                    });
                                                }
                                            })
                                            .catch((error): void => {
                                                if (this.mounted) {
                                                    Modal.error({
                                                        width: 800,
                                                        title: 'Could not synchronize the repository',
                                                        content: error.toString(),
                                                    });

                                                    this.setState({
                                                        repositoryStatus: '!sync',
                                                    });
                                                }
                                            });
                                    }}
                                >
                                    <WarningOutlined />
                                    Synchronize
                                </Tag>
                            )}
                        </Paragraph>
                        <Text strong className='cvat-text-color'>Using format: </Text>
                        <Space>
                            <Select onChange={this.onChangeFormatValue} className='cvat-repository-format-select' value={format}>
                                {
                                    dumpers.map((dumper: any) => (
                                        <Option key={dumper.name} value={dumper.name}>{dumper.name}</Option>
                                    ))
                                }
                            </Select>
                            <Checkbox onChange={this.onChangeLFSValue} checked={lfs}>
                                Large file support
                            </Checkbox>
                        </Space>
                    </Col>
                </Row>
            )
        );
    }

cvat-ui/src/components/task-page/styles.scss

.cvat-dataset-repository-url {
    > a {
        margin-right: $grid-unit-size;
    }

    > .ant-typography {
        > .ant-tag {
            margin-left: $grid-unit-size;

            > span[role='img'] {
                margin-right: $grid-unit-size;
            }
        }

        > .ant-tag-red {
            user-select: none;
            opacity: 0.4;

            &:hover {
                opacity: 0.8;
            }

            &:active {
                opacity: 1;
            }
        }
    }
}

nmanovic · 2022-01-26T13:38:40Z

@PMazarovich , thanks for all your time and contribution. Will you be able to continue improving the PR?

PMazarovich · 2022-01-26T13:43:04Z

@nmanovic , yes, I'll continue approximately next week

PMazarovich · 2022-01-31T15:04:44Z

@bsekachev PR fixed as you have suggested.

cvat-ui/src/components/task-page/details.tsx

cvat-ui/src/containers/task-page/details.tsx

cvat-ui/src/utils/git-utils.ts

cvat-ui/src/components/task-page/details.tsx

bsekachev

Thank your patience and the contribution, works like a charm!

bsekachev · 2022-02-18T11:44:29Z

@nmanovic

Conflict has been resolved.

nmanovic · 2022-02-25T07:58:38Z

cvat/apps/dataset_repo/views.py

@@ -85,6 +86,35 @@ def get_repository(request, tid):

        return HttpResponseBadRequest(str(ex))

+@login_required
+@permission_required(perm=['engine.task.access'],


@PMazarovich , the file doesn't follow the new permission system. I will merge it as is and create an issue to update the app in accordance with IAM in the PR: #3788.

@PMazarovich , please let us know if you can help us to update the Django app.

nmanovic · 2022-02-25T08:47:16Z

Create an issue to improve the feature: #4386

nmanovic · 2022-02-25T08:49:05Z

@PMazarovich , after I change the format, it is impossible to sync repo immediately. Need to change some annotations to do that. In any case I will merge the PR because it works in general. I hope you will help us to improve the feature.

Thanks for the contribution!

nmanovic

LGTM

nmanovic · 2022-02-25T16:00:10Z

@TOsmanov , could you please document changes in the PR? Now it is possible to specify the dataset format for dataset repository and update it on the task page.

TOsmanov · 2022-02-28T04:40:40Z

@TOsmanov , could you please document changes in the PR? Now it is possible to specify the dataset format for dataset repository and update it on the task page.

@nmanovic , yes, sure

PMazarovich requested review from bsekachev and nmanovic as code owners November 9, 2021 11:18

bsekachev self-assigned this Nov 12, 2021

bsekachev reviewed Nov 12, 2021

View reviewed changes

PMazarovich force-pushed the editable_git branch 7 times, most recently from 3bd8d16 to a72c087 Compare December 6, 2021 16:05

PMazarovich force-pushed the editable_git branch from a72c087 to 86e6c30 Compare December 6, 2021 16:25

bsekachev reviewed Jan 10, 2022

View reviewed changes

PMazarovich force-pushed the editable_git branch 2 times, most recently from 49e1596 to a36ba44 Compare January 31, 2022 14:09

Editable git

3351dfd

PMazarovich force-pushed the editable_git branch from a36ba44 to 3351dfd Compare January 31, 2022 14:52

bsekachev reviewed Feb 4, 2022

View reviewed changes

Boris Sekachev added 4 commits February 4, 2022 11:13

Update cvat-ui/src/utils/git-utils.ts

c603af2

Update cvat-ui/src/components/task-page/details.tsx

580f349

Update cvat-ui/src/containers/task-page/details.tsx

b1ff791

Update cvat-ui/src/components/task-page/details.tsx

bda6cf2

bsekachev previously approved these changes Feb 4, 2022

View reviewed changes

Merge branch 'develop' into editable_git

d25bc43

bsekachev dismissed their stale review via d25bc43 February 18, 2022 11:42

nmanovic reviewed Feb 25, 2022

View reviewed changes

nmanovic approved these changes Feb 25, 2022

View reviewed changes

nmanovic merged commit cca2586 into cvat-ai:develop Feb 25, 2022

TOsmanov mentioned this pull request Mar 1, 2022

Update documentation. Add information about git flexibility after task creation #4401

Merged

8 tasks

nmanovic mentioned this pull request Mar 4, 2022

Release v2.0.0 #4422

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Git flexibility after task creation #3886

Git flexibility after task creation #3886

PMazarovich commented Nov 9, 2021 •

edited

Loading

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev Nov 12, 2021

bsekachev commented Nov 12, 2021

PMazarovich commented Nov 26, 2021

PMazarovich commented Dec 6, 2021

bsekachev Jan 10, 2022

bsekachev Jan 10, 2022

bsekachev Jan 10, 2022

PMazarovich Jan 18, 2022

bsekachev Jan 19, 2022

bsekachev Jan 10, 2022

bsekachev Jan 10, 2022

bsekachev commented Jan 10, 2022 •

edited

Loading

bsekachev commented Jan 10, 2022

nmanovic commented Jan 26, 2022

PMazarovich commented Jan 26, 2022

PMazarovich commented Jan 31, 2022

bsekachev left a comment

bsekachev commented Feb 18, 2022

nmanovic Feb 25, 2022

nmanovic Feb 25, 2022

nmanovic commented Feb 25, 2022

nmanovic commented Feb 25, 2022

nmanovic left a comment

nmanovic commented Feb 25, 2022

TOsmanov commented Feb 28, 2022

	this.setState({lfs: value.target.checked});
	this.setState({ lfs: value.target.checked });

	this.setState({format:value});
	this.setState({ format: value });

	<Select onChange={this.onChangeFormatValue} className='cvat-repository-format-select' initialValue={format} defaultValue={format}>
	<Select onChange={this.onChangeFormatValue} className='cvat-repository-format-select' value={format}>

Git flexibility after task creation #3886

Git flexibility after task creation #3886

Conversation

PMazarovich commented Nov 9, 2021 • edited Loading

Motivation and context

How has this been tested?

Checklist

License

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsekachev commented Nov 12, 2021

PMazarovich commented Nov 26, 2021

PMazarovich commented Dec 6, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bsekachev commented Jan 10, 2022 • edited Loading

bsekachev commented Jan 10, 2022

nmanovic commented Jan 26, 2022

PMazarovich commented Jan 26, 2022

PMazarovich commented Jan 31, 2022

bsekachev left a comment

Choose a reason for hiding this comment

bsekachev commented Feb 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nmanovic commented Feb 25, 2022

nmanovic commented Feb 25, 2022

nmanovic left a comment

Choose a reason for hiding this comment

nmanovic commented Feb 25, 2022

TOsmanov commented Feb 28, 2022

PMazarovich commented Nov 9, 2021 •

edited

Loading

bsekachev commented Jan 10, 2022 •

edited

Loading