Skip to content

Commit

Permalink
build: Fail PR if requirements files are inconsistent (openedx#32057)
Browse files Browse the repository at this point in the history
We have a few situations where requirements files can become
inconsistent or cause unnecessary review churn in later `make upgrade`
runs either due to manual editing, inconsistent environments, or
incorrectly specified git dependencies. This will produce a failing
check for any PR that does not produce a clean run of `make
compile-requirements` on Linux.

Addresses openedx#31372
  • Loading branch information
timmc-edx authored Aug 15, 2023
1 parent fa22e01 commit 595c94b
Show file tree
Hide file tree
Showing 2 changed files with 114 additions and 3 deletions.
69 changes: 69 additions & 0 deletions .github/workflows/check-consistent-dependencies.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
# Rejects PR if requirements files are inconsistent.
#
# This will produce a failing check for any PR that does not produce a
# clean run of `make compile-requirements` on Linux.

name: Consistent Python dependencies

on:
pull_request:
paths:
- 'requirements/**'
push:
branches:
- master
paths:
- 'requirements/**'

defaults:
run:
shell: bash # strict bash

jobs:
check-requirements:
name: Compile requirements
runs-on: ubuntu-22.04

steps:
- uses: actions/checkout@v3

- uses: actions/setup-python@v4
with:
python-version: '3.8'

- run: |
make compile-requirements
- name: Fail if compiling requirements caused changes
run: |
SUMMARY_HELP=$(cat <<'EOMARKDOWN'
# Inconsistent Python dependencies
It appears that the Python dependencies in this PR are inconsistent: A re-run of
`make compile-requirements` produced changes. This might mean that your PR would
fail to deploy properly in production, or could have inconsistent behavior for
developers.
Please see the requirements README for information on how to resolve this:
https://github.com/openedx/edx-platform/blob/master/requirements/README.rst#inconsistent-dependencies
EOMARKDOWN
)
make_summary () {
echo "$SUMMARY_HELP"
echo
echo "----"
echo
echo "Diff follows:"
echo
echo '```'
git diff || true
echo '```'
}
git diff --quiet --exit-code || {
# Job Summaries are cool, but echo to the job log as well, because
# that's where the PR checks will actually link to.
make_summary | tee -a $GITHUB_STEP_SUMMARY
exit 1
}
48 changes: 45 additions & 3 deletions requirements/README.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
Requirements/dependencies
=========================
#########################

These directories specify the Python (and system) dependencies for the LMS and Studio.

Expand All @@ -12,15 +12,22 @@ directly in the requirements directory.)

.. _OEP-18: https://github.com/openedx/open-edx-proposals/blob/master/oeps/oep-0018-bp-python-dependencies.rst

Upgrading all dependencies
**************************

You can use the `upgrade-requirements Github Workflow <https://github.com/openedx/edx-platform/actions/workflows/upgrade-python-requirements.yml>`_ to make a PR that upgrades as many packages as possible.

Upgrading just one dependency
-----------------------------
*****************************

Want to upgrade just *one* dependency without pulling in other upgrades? You can `run the upgrade-one-python-dependency.yml workflow <https://github.com/openedx/edx-platform/actions/workflows/upgrade-one-python-dependency.yml>`_ to have a pull request made against a branch of your choice.

Or, if you need to do it locally, you can use the ``upgrade-package`` make target directly. For example, you could run ``make upgrade-package package=ecommerce``. But the GitHub workflow is likely easier.

If your dependency is pinned in constraints.txt, you'll need to enter an explicit version number in the appropriate field when running the workflow; this will include an update to the constraint file in the resulting PR.

Downgrading a dependency
------------------------
************************

If you instead need to surgically *downgrade* a dependency:

Expand All @@ -32,3 +39,38 @@ If you instead need to surgically *downgrade* a dependency:
2. Run ``make compile-requirements``

This is considerably safer than trying to manually edit the ``*.txt`` files, which can easily result in incompatible dependency versions.

Inconsistent dependencies
*************************

You might be directed to this section if a PR check for consistent dependencies has failed.

Did you run ``make upgrade`` or ``make compile-requirements`` on a Mac directly?
================================================================================

Some packages have different dependencies on Mac vs. Linux. Usually this is not relevant in production (they generally have to do with desktop integrations of developer tools) but this does cause "churn" and make it harder to review PRs when dependencies are alternatingly recompiled on Mac and Linux. As edx-platform runs on Linux, we want to ensure that dependencies are compiled for that platform.

Solutions for Mac users:

- Use the workflow described in `Upgrading just one dependency`_.
- You can run ``make lms-shell`` in devstack to get a Linux environment for more complicated operations.

Did you hand-edit the .txt files?
=================================

Hand-editing the .txt requirements files often leads to dependency conflicts, failed deployments, or outages. It's easy to forget to update all the locations where a requirement appears, and it's often not feasible to track down all of the transitive dependencies of the package you want to upgrade.

Luckily, we have simple runbooks for upgrading or downgrading a single package, which are the most common cases:

- `Upgrading just one dependency`_
- `Downgrading a dependency`_

Is there an unpinned git dependency?
====================================

If the diff relates to a dependency that is installed from git rather than from PyPI (such as being a transitive dependency of anything in github.in), check whether any of the dependencies in github.in has failed to pin a specific commit. We want to have as few of these dependencies as possible, as they're a maintenance and performance problem, and there are important instructions at the top of that file for how to manage them.

Help, I didn't change any dependencies, and this is still failing!
==================================================================

It's possible that someone introduced an inconsistency on the master branch, in which case please submit a new PR off of master after running ``make compile-requirements`` (but see notes above for Mac users). Or perhaps your branch was made while there was such an inconsistency, in which case please rebase onto master or merge down from master to your branch.

0 comments on commit 595c94b

Please sign in to comment.