docs: amend OEP-18 to further discourage VCS URL dependencies - #390
docs: amend OEP-18 to further discourage VCS URL dependencies#390kdmccormick wants to merge 1 commit into
Conversation
| to accomodate a forward fix. If a repository maintainer is unresponsive, this | ||
| issue can be escalated to tCRIL, which can cut an emergency release if necessary. If | ||
| the repository is outside of the Open edX project, then the repository can be forked, and | ||
| the package re-released to PyPI under a new name. |
There was a problem hiding this comment.
One of the most common historical causes of VCS dependencies is needing support for a new Python or Django version for a named Open edX release before upstream is ready to merge and/or release it (because Open edX named releases come out before Python/Django EOL dates). The absolute number is still small, but probably happens about 5 times across repos each Django upgrade cycle.
We can technically release forks of these to PyPI each time it happens, but just want to make clear that this will cause a fair bit of overhead each time it happens; we'd want a pretty solid workflow for this, and the "temporary" forks will pretty much sit in PyPI until we're willing to break all installations of the named release that needed it.
| <https://github.com/openedx/edx-platform/blob/open-release/nutmeg.2/requirements/edx/github.in#L54-L77>`_ | ||
| Following ``git blame`` shows that the current VCS URL dependencies | ||
| exist simply because it took fewer steps to set them up than it would have to | ||
| install them from PyPI. |
There was a problem hiding this comment.
A few other interesting use cases that have come up in the past:
- A dependency suddenly deletes all of its old PyPI releases, as in fix: use github version of py2neo openedx-platform#27168 . This removes the "downgrade" option and makes it critical to fix the dependency ASAP because it's breaking all deployments and installations.
- Something about codejail is preventing it from working correctly when installed from PyPI; Arbi-BOM has a new idea regarding what causes that and is currently looking into it, but it's not fixed yet. https://openedx.atlassian.net/browse/BOM-2481
- We had to temporarily fork celery just to get useful logging data to diagnose a recurring problem in production: Adding custom fork celery with better logging. openedx-platform#25649
- Sometimes upstream drops previous LTS Django support before releasing a version with the new LTS Django support, like in Upgrade django-babel-underscore BOM-1069 openedx-platform#23257 .
I don't want to argue that installing from VCS is the only way to solve problems like these. I just want to make sure that we have a replacement process that allows us to very quickly deploy a specific commit from a dependency before we eliminate this as an option.
There was a problem hiding this comment.
Good to know @jmbowman . I'm glad you saw this draft PR and brought these up before I went any further with this.
I believe the codejail issue was an instance of an incorrect setup.py. I don't have good alternative workflows for the other instances you brought up, though.
To be honest, I don't think BOM teams adding temporary URL dependencies in order to speed up upgrades and handle critical issues is the problem here. The problem is when URL dependencies are added as a quick-and-dirty way of getting a feature out the door. So, maybe the solution isn't to forbid VCS URLs altogether. Perhaps it'd be better to, for example, use CODEOWNERS to require review from one of the BOM teams when editing github.in, helping ensure the file isn't misused.
There was a problem hiding this comment.
I definitely agree that there are things in github.in that have stayed there far too long. Like, there's never been a proper release of our drag & drop assessment XBlock. But we may need something more like a repo health check and a followup process for remediation than an outright ban.
|
Based on Jeremy's feedback, I will be amending this update to be less extreme. I'll leave room in the OEP for using GitHub-based dependencies in the cases he listed. And instead of a blocking linter, I'm going to look into using CODEOWNERS or a repo health check. |
|
Superseded by #450 |
Part of openedx-unsupported/wg-developer-experience#84
Related to openedx/.github#26