NASA-PDS / roundup-action

Do a "roundup", a/k/a PDS-style continuous integration and delivery
Apache License 2.0
1 stars 4 forks source link

Resolve #79 #80

Closed nutjob4life closed 2 years ago

nutjob4life commented 2 years ago

🗒️ Summary

Merge this if you dare to resolve #79.

We don't mess around with PYTHONPATHs since that's what virtual environments are for. By installing a package X in venv V, then when you invoke any Python executable from V, X will be one of the packages available to that executable.

Now, the Roundup sets up a venv for pds-doi-service, essentially doing:

cd pds-doi-service
python -m venv --system-site-packages venv  # 1
export PATH=venv/bin:${PATH}                # 2

Then it installs the package being "rounded up", i.e., pds-doi-service, essentially:

pip install --editable .[dev]  # 3

Here ③'s pip is venv/bin/pip. So later when it does a unit test, it executes:

tox -e py39

Because of ②, the very first tox found is venv/bin/tox, and since that tox comes from the virtual environment created for the package being integrated in step ①, then it has access to all the source in pds-doi-service because of step ③.

Now, it tries to make the documentation by running:

sphinx-build -a -b html docs/source docs/build

And gets

docs/source/api/index.rst:9: WARNING: Failed to import "create_parser" from "pds_doi_service.core.actions.action".
No module named 'pds_doi_service'

This step tries to reference some of the code in pds_doi_service.core.actions.action.create_parser. It failed to do so, though, because sphinx-build in this case was

/usr/local/bin/sphinx-build

and not venv/bin/sphinx-build. Why would this be the case? After all, in a brand new Python with no other dependencies, when you run pip install --editable .[dev] in pds-doi-service, you get venv/bin/sphinx-build!

The key is the flag in step ①: --system-site-packages. Because sphinx is in the system Python, in step ③ it won't get re-installed as a dependency of pds_doi_service. But because this is an executable run outside of the venv for the package being "rounded up", it doesn't have access to that package.

We can remove --system-site-packages:

cd pds-doi-service
python -m venv venv            # 4: No more access to the system Python's pre-baked packages
export PATH=venv/bin:${PATH}
pip install --editable .[dev]  # 5

Now the path for sphinx-build is venv/bin/sphinx-build. This comes from the venv thanks to ④, and so now

sphinx-build -a -b html docs/source docs/build

produces no warnings. However, we've got a new problem! Step ⑤ now takes 23 minutes to run on my MacBook Pro, which is 4+ hours on GitHub Actions. That's terrible!

Why is this? Well, the key is --system-site-packages! We included packages like numpy, pandas, etc., in the base image for the Roundup because those things take forever to install. By allowing the package that's being "rounded up" to have access to the system Python's packages, we can just use the pre-installed numpy, pandas, etc. It's a huge time-saver.

The problem, though, is that sphinx is also pre-installed. What a dilemma!

But there's a workaround:

cd pds-doi-service
python -m venv --system-site-packages venv
export PATH=venv/bin:${PATH}
pip install --ignore-installed sphinx           # 6
pip install --editable .[dev]
sphinx-build -a -b html docs/source docs/build  # 7

Now step ⑥ forces our venv to have its own Sphinx, so step ⑦'s sphinx-build will run out of the venv, and as such it'll have access to the package being rounded up. No more warnings.

⚙️ Test Data and/or Report

Omitted: pull-request body too long

♻️ Related Issues

MJJoyce commented 2 years ago

Well good job getting that figured out

jordanpadams commented 2 years ago

😮 nice work