feat: use a virtualenv around all python Make targets #367
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces virtualenv management for all Python-based Make targets to create isolated development environments. The changes update the Makefile to automatically create and use a Python 3.11 virtualenv for all operations, with automatic installation of Python 3.11 on Ubuntu systems that lack it.
Changes:
- Modified Makefile to create and use a virtualenv for all Python operations
- Updated all Python-related Make targets to use virtualenv-based executables
- Upgraded Python dependencies (boto3, botocore, cryptography, jmespath, packaging, pycparser, pyparsing, wheel, setuptools)
- Added CLAUDE.md documentation file explaining the project structure and development workflow
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| Makefile | Implemented virtualenv creation and usage across all Python targets, added automatic Python 3.11 installation for Ubuntu systems |
| requirements/test.txt | Updated dependency versions (boto3, botocore, cryptography, jmespath, packaging, pycparser, pyparsing) |
| requirements/pip_tools.txt | Updated packaging and wheel versions, adjusted dependency tree formatting |
| requirements/pip.txt | Updated wheel, setuptools versions and added packaging dependency |
| requirements/base.txt | Updated boto3, botocore, and jmespath versions |
| CLAUDE.md | Added comprehensive documentation about project structure, commands, and architecture |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
iloveagent57
force-pushed
the
aed/virtualenv-maybe
branch
from
584ae69 to
0911a90
Compare
iloveagent57
force-pushed
the
aed/virtualenv-maybe
branch
from
0911a90 to
3ede2fb
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @if [ ! -d "$(VENV)" ]; then \ | ||
| echo "Creating virtual environment in $(VENV)"; \ | ||
| python3.11 -m venv $(VENV); \ | ||
| $(PIP) install --upgrade pip; \ | ||
| else \ | ||
| echo "Virtual environment $(VENV) already exists, skipping creation"; \ | ||
| fi |
There was a problem hiding this comment.
In the venv target, when Python 3.11 is not initially found (PYTHON311 is empty) and apt-get installation is triggered, the shell check on line 10 (PYTHON311 := $(shell command -v python3.11 2> /dev/null)) is evaluated once at the start of the Makefile execution. After installing Python 3.11 via apt-get (lines 39-43), the PYTHON311 variable will still be empty because Make variables are expanded before target execution. This means line 50 will always execute python3.11 -m venv even when PYTHON311 was initially empty, which could fail if the installation didn't complete successfully or if the PATH hasn't been updated. Consider re-checking for python3.11 availability in the shell script on line 48-54, or ensure proper error handling.
No description provided.