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 creation of tasks with Git repositories via the SDK #5409

Merged
merged 7 commits into from
Dec 5, 2022

Conversation

SpecLad
Copy link
Contributor

@SpecLad SpecLad commented Dec 1, 2022

Motivation and context

Fixes #4365.

How has this been tested?

SDK unit tests + I did some Git-related operations via the UI to make sure they weren't broken.

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
Copy link
Contributor Author

SpecLad commented Dec 1, 2022

FYI, there are 4 independent issues I had to fix to make this work; see the individual commits for more explanation.

@SpecLad SpecLad force-pushed the cvat-sdk-git branch 2 times, most recently from 4fcc9dd to 040256f Compare December 1, 2022 18:16
The main goal is to align authentication logic between `dataset_repo` and
the rest of the API. In particular, the common logic disables CSRF checks
for queries that have token authentication, which will allow the Git API
to be used from the SDK.

The interactive HTML API pages that DRF provides are pretty nice too.

For one of the endpoints, fix the UI to pass the correct Content-Type,
otherwise, DRF rejects the request.
There's already format autodetection code in `CVATGit.initial_create`,
might as well let the client use it by not specifying a format.
The various server/worker processes may be running in separate containers,
in which case `~/.ssh/ssh.pid` may refer to a different file in each container.
Use `~/keys/ssh.pid` instead, because that is mounted from a shared volume.
@bsekachev
Copy link
Member

Fix for failed 3D test: 3f097f5
Explanation: #5407 (comment)

Copy link
Contributor

@zhiltsov-max zhiltsov-max left a comment

Choose a reason for hiding this comment

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

The PR looks good to me. Maybe, we could add some info about the expected format in the CLI output and in the task creating function docs. A link to the description page may be enough for this .

@bsekachev bsekachev mentioned this pull request Dec 2, 2022
@SpecLad
Copy link
Contributor Author

SpecLad commented Dec 2, 2022

Maybe, we could add some info about the expected format in the CLI output and in the task creating function docs.

Maybe, but that's not related to the bug. 🙂 I didn't change the "face" of the API, so I don't think documenting it should be part of this PR.

@nmanovic nmanovic merged commit 192fd72 into cvat-ai:develop Dec 5, 2022
@SpecLad SpecLad deleted the cvat-sdk-git branch March 16, 2023 18:00
mikhail-treskin pushed a commit to retailnext/cvat that referenced this pull request Jul 1, 2023
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.

Create task with git repository
4 participants