-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Remove the Git repository synchronization functionality #6904
Conversation
5acf843
to
9c9726f
Compare
/check |
❌ Some checks failed |
This functionality has accumulated significant technical debt: * Most importantly, it does not use the current authorization system, rendering it accessible only for admin users. * It doesn't follow the regular API conventions, and is not visible in the API schema. This necessitates special code in the SDK. * The initialization code in `base.py` is not safe when multiple instances of the server start at the same time (each instance may end up generating its own key). The team has decided that the cost of fixing these issues outweighs the benefit of the functionality, so remove it.
9c9726f
to
69debdf
Compare
/check |
✔️ All checks completed successfully |
Codecov Report
@@ Coverage Diff @@
## develop #6904 +/- ##
=========================================
Coverage 82.52% 82.52%
=========================================
Files 370 360 -10
Lines 39869 38908 -961
Branches 3560 3544 -16
=========================================
- Hits 32902 32110 -792
+ Misses 6967 6798 -169
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This functionality has accumulated significant technical debt: * Most importantly, it does not use the current authorization system, rendering it accessible only for admin users. * It doesn't follow the regular API conventions and is not visible in the API schema. This necessitates a special code in the SDK. * The initialization code in `base.py` is not safe when multiple instances of the server starts at the same time (each instance may end up generating its own key). The team has decided that the cost of fixing these issues outweighs the benefit of the functionality, so remove it.
Hello! |
@PMazarovich , you are the only known user of the feature. Could you please describe how have you used the feature? |
@nmanovic here it is:
|
IMHO it was a nice automatic backup feature and I would prefer it to other methods like manually created backups by admins or even users. I understand your decision "cost of fixing vs. benefit of functionality" but nevertheless it was a useful feature and I would vote for it. :-) |
Can it be implemented using web hooks? https://opencv.github.io/cvat/docs/administration/advanced/webhooks/ |
This was only used by the Git synchronization functionality, which was removed in #6904.
Motivation and context
This functionality has accumulated significant technical debt:
Most importantly, it does not use the current authorization system,
rendering it accessible only for admin users.
It doesn't follow the regular API conventions, and is not visible in the API
schema. This necessitates special code in the SDK.
The initialization code in
base.py
is not safe when multiple instances ofthe server start at the same time (each instance may end up generating its
own key).
The team has decided that the cost of fixing these issues outweighs the benefit
of the functionality, so remove it.
How has this been tested?
Checklist
develop
branch[ ] I have linked related issues (see GitHub docs)(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.