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

Cannot run make requirements from an empty virtualenv for cookiecutter-django-ida #317

Closed
2 tasks done
pshiu opened this issue Apr 12, 2023 · 5 comments
Closed
2 tasks done
Assignees

Comments

@pshiu
Copy link
Contributor

pshiu commented Apr 12, 2023

The Makefile target piptools, which installs pip-sync, was removed from cookiecutter-django-ida (but not other cookiecutters) in [link to line]. This means that make requirements in a clean virtualenv will fail for that cookiecutter's outputs.

The other cookiecutters vary in how they install the pip-tools package.

Acceptance criteria:

@pshiu
Copy link
Contributor Author

pshiu commented Apr 12, 2023

How each repo handles installing pip-tools / pip-sync via make requirements now:

  • cookiecutter-django-app uses a piptools recipe
  • cookiecutter-django-ida doesn't install it
  • cookiecutter-xblock doesn't install it
  • python-template runs pip install -r requirements/pip.txt and pip install -r requirements/pip-tools.txt in the make requirements recipe

Manually edited shell search:

edx-cookiecutters % git grep -n -A5 '^requirements:' **/Makefile
cookiecutter-django-app/{{cookiecutter.repo_name}}/Makefile:63:requirements: piptools ## install development environment requirements
cookiecutter-django-app/{{cookiecutter.repo_name}}/Makefile-64- pip-sync -q requirements/dev.txt requirements/private.*
--
cookiecutter-django-ida/{{cookiecutter.repo_name}}/Makefile:38:requirements: clean_pycrypto dev_requirements ## sync to default requirements
--
cookiecutter-xblock/{{cookiecutter.repo_name}}/Makefile:45:requirements: piptools## install development environment requirements
cookiecutter-xblock/{{cookiecutter.repo_name}}/Makefile-46-     pip-sync -q requirements/dev.txt requirements/private.*
--
python-template/{{cookiecutter.placeholder_repo_name}}/Makefile:61:requirements: ## install development environment requirements
python-template/{{cookiecutter.placeholder_repo_name}}/Makefile-62-     pip install -r requirements/pip.txt
python-template/{{cookiecutter.placeholder_repo_name}}/Makefile-63-     pip install -r requirements/pip-tools.txt
python-template/{{cookiecutter.placeholder_repo_name}}/Makefile-64-     pip-sync requirements/dev.txt requirements/private.*

@timmc-edx
Copy link
Contributor

Hmm! Yeah, make requirements should be installing the pip and piptools pinning files first, one way or another. (There are a few ways that we have gone about this in various repos.) I also would have expected CI to catch this, as we do have automated tests that use the cookiecutters and then run tests in the new virtualenv. Perhaps that virtualenv is dirty somehow, though? I'll add that to the acceptance criteria.

@dianakhuang dianakhuang moved this to Prioritized in Arch-BOM Apr 14, 2023
@timmc-edx timmc-edx moved this from Prioritized to In Progress in Arch-BOM Apr 24, 2023
@timmc-edx timmc-edx self-assigned this Apr 24, 2023
@timmc-edx
Copy link
Contributor

@pshiu I think the original scope of the issue is complete; cookiecutter-django-ida now has a working make requirements. However, I did want to check on something -- one thing about the cookiecutters is that you need to run make upgrade after using the cookiecutter, which is annoying manual step, and would also result in make requirements not working. Is that something you ran into? I have it listed in the A/C of this issue to look into making that automated (probably as a separate issue), but wanted to check first if that was something you encountered as a pain point.

@pshiu
Copy link
Contributor Author

pshiu commented May 9, 2023

@timmc-edx Hooray! Thanks for taking on this work!

I never found make requirements inoperable after a make upgrade, but I was so frustrated back then & was so furiously running manual installs of pip that I'm sure I might have missed that being the case.

@timmc-edx timmc-edx removed the status in Arch-BOM May 10, 2023
@timmc-edx timmc-edx moved this to In Progress in Arch-BOM May 10, 2023
@timmc-edx
Copy link
Contributor

Filed #337 for automatically running make upgrade

@github-project-automation github-project-automation bot moved this from In Progress to Done in Arch-BOM May 10, 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

No branches or pull requests

2 participants