[build] use bazel to update python requirements

[titusfortner]

User description

Java explicitly updates versions in pre-release workflow
Ruby & Rust do so when their versions are updated.
Goal is to have something automated for python as well.

💥 What does this PR do?

• updates the shell script @cgoldberg created to a python script
• set the python script to run with a bazel target
• add the bazel target to py:update rake task
• add py:update task to pre-release workflow

🔧 Implementation Notes

Tried to follow the existing approach, just with bazel's python

---

PR Type

Enhancement, Tests

---

Description

• Replace shell script with Python implementation for updating dependencies

• Integrate Python dependency updates into pre-release workflow automation

• Add Bazel target and Rake task for streamlined dependency management

• Preserve package extras and formatting in requirements file

---

Diagram Walkthrough

flowchart LR
  A["update_py_dependencies.sh<br/>Shell Script"] -->|"Replace with"| B["update_py_deps.py<br/>Python Script"]
  B -->|"Bazel Target"| C["//scripts:update_py_deps"]
  C -->|"Rake Task"| D["py:update"]
  D -->|"Pre-release Workflow"| E["Automated Updates"]

Loading

File Walkthrough

	Relevant files
Enhancement	update_py_deps.py New Python dependency update script                                            scripts/update_py_deps.py New Python script to update py/requirements.txt with latest compatible versions Creates temporary virtual environment and uses pip to resolve package versions Preserves original formatting, comments, and package extras like urllib3[socks] Provides detailed output of version changes and next steps +112/-0  Rakefile Add Rake task for Python dependency updates                            Rakefile Add new py:update Rake task that executes Bazel targets Runs update_py_deps script and requirements.update target Automatically stages updated requirements files for git commit +8/-0
Cleanup	update_py_dependencies.sh Remove legacy shell script                                                              scripts/update_py_dependencies.sh Removed shell script implementation Functionality migrated to Python-based update_py_deps.py +0/-81
Configuration changes	pre-release.yml Integrate Python updates into pre-release workflow              .github/workflows/pre-release.yml Add py:update task to pre-release workflow after Maven updates Consolidate commit message for all dependency version updates Enables automated Python dependency updates in release pipeline +4/-2      BUILD.bazel Add Bazel target for dependency updater                                    scripts/BUILD.bazel Add py_binary target for update_py_deps script Enables execution via bazel run //scripts:update_py_deps +5/-0

---

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Supply chain execution Description: The script runs pip install on requirement names during automation, which can execute arbitrary code from downloaded distributions (e.g., via build backends or legacy setup.py) and introduces a supply-chain risk if a dependency is compromised/typosquatted or if an attacker can alter py/requirements.txt in a PR that later runs in a privileged workflow. update_py_deps.py [54-64] Referred Code # Install packages (with extras) to let pip resolve versions install_names = [p[1] for p in packages] # name_with_extras print(f"Installing {len(install_names)} packages...") result = subprocess.run( [str(pip), "install", "-q"] + install_names, capture_output=True, text=True, ) if result.returncode != 0: print(f"Error installing packages:\n{result.stderr}") return
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Unhandled subprocess failures: Several subprocess.run(..., check=True) calls can raise uncaught exceptions (and other failures return without a non-zero exit code), preventing graceful handling and actionable error context. Referred Code if not requirements_file.exists(): print(f"Error: {requirements_file} not found") return print(f"Checking {requirements_file}") # Parse current requirements preserving original format current_lines = requirements_file.read_text().strip().split("\n") packages = [] # (original_line, package_name_with_extras, package_name_normalized) for line in current_lines: line = line.strip() if line and not line.startswith("#") and "==" in line: name_with_extras, version = line.split("==", 1) # Normalize: remove extras for pip queries name_normalized = name_with_extras.split("[")[0].lower() packages.append((line, name_with_extras, name_normalized)) with tempfile.TemporaryDirectory() as tmpdir: venv_dir = Path(tmpdir) / "venv" # Create virtual environment ... (clipped 27 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Potential stacktrace exposure: Failures from subprocess.run(..., check=True) are not caught and may surface full stack traces and internal paths in CI logs depending on invocation context. Referred Code print("Creating temporary virtual environment...") subprocess.run( [sys.executable, "-m", "venv", str(venv_dir)], check=True, capture_output=True, ) pip = venv_dir / "bin" / "pip" # Upgrade pip first subprocess.run( [str(pip), "install", "-q", "--upgrade", "pip"], check=True, capture_output=True, ) Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Raw stderr printed: The script prints raw pip stderr to stdout, which may include sensitive repository URLs or credentials depending on configured package indexes in the execution environment. Referred Code print(f"Installing {len(install_names)} packages...") result = subprocess.run( [str(pip), "install", "-q"] + install_names, capture_output=True, text=True, ) if result.returncode != 0: print(f"Error installing packages:\n{result.stderr}") return Learn more about managing compliance generic rules or creating your own custom rules
Update

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Latest suggestions up to b76dbfa

Category	Suggestion	Impact
Possible issue	✅ Propagate failures via exit codes Suggestion Impact: The commit changed error paths from `print(...); return` to raising exceptions (`FileNotFoundError`, `RuntimeError`), which will terminate the script with a non-zero status when unhandled—addressing the core goal of propagating failures (though not using `SystemExit`/stderr exactly as suggested, and the "no updates" success-exit change is not shown here). code diff: if not requirements_file.exists(): - print(f"Error: {requirements_file} not found") - return + raise FileNotFoundError(f"{requirements_file} not found") print(f"Checking {requirements_file}") @@ -60,8 +59,7 @@ text=True, ) if result.returncode != 0: - print(f"Error installing packages:\n{result.stderr}") - return + raise RuntimeError(f"Error installing packages:\n{result.stderr}") Modify the script to exit with a non-zero status code on failures and a zero status code on success. This ensures that errors are correctly propagated when the script is run as part of an automated task. scripts/update_py_deps.py [17-91] if not requirements_file.exists(): - print(f"Error: {requirements_file} not found") - return + print(f"Error: {requirements_file} not found", file=sys.stderr) + raise SystemExit(1) ... if result.returncode != 0: - print(f"Error installing packages:\n{result.stderr}") - return + print(f"Error installing packages:\n{result.stderr}", file=sys.stderr) + raise SystemExit(result.returncode) ... if not updates: print("\nAll packages are up to date!") - return + raise SystemExit(0) [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 8 __ Why: This is a critical fix for correctness, as the script is part of an automated rake task. Using return instead of exiting with a non-zero status code would cause silent failures, making the overall task appear successful even when it's not.	Medium
	Fail on duplicate package entries Add a check to detect and fail on duplicate normalized package names in requirements.txt. This prevents potential corruption of the file during the update process. scripts/update_py_deps.py [25-32] packages = [] # (original_line, package_name_with_extras, package_name_normalized) +seen = set() for line in current_lines: line = line.strip() if line and not line.startswith("#") and "==" in line: name_with_extras, version = line.split("==", 1) # Normalize: remove extras for pip queries name_normalized = name_with_extras.split("[")[0].lower() + if name_normalized in seen: + print( + f"Error: duplicate requirement for '{name_normalized}' detected; " + "cannot safely update pinned versions.", + file=sys.stderr, + ) + raise SystemExit(1) + seen.add(name_normalized) packages.append((line, name_with_extras, name_normalized)) Apply / Chat Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies a potential data corruption bug where duplicate normalized package names could lead to incorrect updates in requirements.txt. Adding a check to prevent this improves the script's robustness and prevents subtle errors.	Medium
	Validate dependency consistency after upgrades After installing packages, run pip check to verify dependency consistency within the temporary environment. If conflicts are found, the script should fail with an error to prevent generating a broken requirements.txt. scripts/update_py_deps.py [73] installed = {pkg["name"].lower(): pkg["version"] for pkg in json.loads(result.stdout)} +# Verify dependency consistency +check = subprocess.run( + [str(pip), "check"], + capture_output=True, + text=True, +) +if check.returncode != 0: + print(f"Dependency conflicts detected:\n{check.stdout}{check.stderr}", file=sys.stderr) + raise SystemExit(check.returncode) + Apply / Chat Suggestion importance[1-10]: 7 __ Why: This is a valuable suggestion that improves the reliability of the dependency update process. By running pip check, the script can proactively identify and report dependency conflicts, preventing the generation of an inconsistent requirements.txt file that could cause issues later.	Medium
Learned best practice	Validate generated files exist After running Bazel, verify py/requirements.txt and py/requirements_lock.txt exist (and ideally are non-empty) before git add, and raise a clear error if not. rake_tasks/python.rake [126-129] Bazel.execute('run', [], '//scripts:update_py_deps') Bazel.execute('run', [], '//py:requirements.update') -SeleniumRake.git.add('py/requirements.txt') -SeleniumRake.git.add('py/requirements_lock.txt') +%w[py/requirements.txt py/requirements_lock.txt].each do |path| + raise "Expected file not found: #{path}" unless File.exist?(path) + SeleniumRake.git.add(path) +end + Apply / Chat Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries by checking expected outputs exist before acting on them.	Low
Update

---

Previous suggestions

✅ Suggestions up to commit e791f86

Category	Suggestion	Impact
High-level	Use a standard tool for dependency management Replace the custom Python dependency update script with a standard tool like pip-tools. This change would simplify the dependency management process and improve long-term maintainability. Examples: scripts/update_py_deps.py [1-112] #!/usr/bin/env python import json import subprocess import sys import tempfile from pathlib import Path # Updates py/requirements.txt with latest compatible versions using pip # Run with: bazel run //scripts:update_py_deps ... (clipped 102 lines) Solution Walkthrough: Before: # scripts/update_py_deps.py def main(): requirements_file = "py/requirements.txt" current_lines = requirements_file.read_text().split("\n") packages = parse_packages(current_lines) with TemporaryDirectory() as tmpdir: # Create venv subprocess.run(["python", "-m", "venv", ...]) pip = ... # Install packages to let pip resolve latest versions subprocess.run([pip, "install", ...]) # Get new versions from the venv result = subprocess.run([pip, "list", "--format=json"], ...) installed_versions = {pkg["name"]: pkg["version"] for pkg in json.loads(result.stdout)} # Rebuild requirements.txt with new versions, preserving format updated_lines = build_new_requirements(packages, installed_versions) requirements_file.write_text("\n".join(updated_lines)) After: # py/requirements.in (new file) # List top-level dependencies here. # `pip-compile` will pin these and their sub-dependencies. urllib3[socks] trio trio-websocket ... # Rakefile (or CI script) # The update command simplifies to: # This replaces the custom script. sh "pip-compile --upgrade --output-file=py/requirements.txt py/requirements.in" # The custom update_py_deps.py script is no longer needed. Suggestion importance[1-10]: 9 __ Why: The suggestion correctly identifies that the custom script reinvents functionality provided by a standard tool (pip-tools), and adopting it would significantly improve maintainability and align with Python best practices.	High
Possible issue	Fix package update logic Modify the package update logic to first install the currently pinned versions from requirements.txt before attempting to upgrade them. This ensures the dependency resolution starts from a known baseline. scripts/update_py_deps.py [54-64] # Install packages (with extras) to let pip resolve versions +print(f"Installing packages from {requirements_file.name}...") +subprocess.run( + [str(pip), "install", "-q", "-r", str(requirements_file)], + check=True, + capture_output=True, +) + install_names = [p[1] for p in packages] # name_with_extras -print(f"Installing {len(install_names)} packages...") +print(f"Upgrading {len(install_names)} packages...") result = subprocess.run( - [str(pip), "install", "-q"] + install_names, + [str(pip), "install", "-q", "--upgrade"] + install_names, capture_output=True, text=True, ) if result.returncode != 0: - print(f"Error installing packages:\n{result.stderr}") + print(f"Error upgrading packages:\n{result.stderr}") return Suggestion importance[1-10]: 8 __ Why: This suggestion correctly identifies a logical flaw in the package update strategy. The current implementation re-resolves dependencies from scratch, while the suggestion proposes a more robust method of first installing pinned versions and then upgrading them, which better reflects an "update" process and aligns with best practices.	Medium
Learned best practice	Validate and harden requirements parsing Restrict updates to valid pinned requirement lines (ignore inline comments, hashes, and URL/option lines) and validate parsed names/versions before using them to prevent corrupting requirements.txt. scripts/update_py_deps.py [24-32] -current_lines = requirements_file.read_text().strip().split("\n") +current_lines = requirements_file.read_text().splitlines() packages = [] # (original_line, package_name_with_extras, package_name_normalized) -for line in current_lines: - line = line.strip() - if line and not line.startswith("#") and "==" in line: - name_with_extras, version = line.split("==", 1) - # Normalize: remove extras for pip queries - name_normalized = name_with_extras.split("[")[0].lower() - packages.append((line, name_with_extras, name_normalized)) +for raw_line in current_lines: + line = raw_line.strip() + if ( + not line + or line.startswith("#") + or line.startswith("-") + or "://" in line + or line.startswith("--") + ): + continue + # drop inline comments (e.g. "pkg==1.2.3 # via ...") + line_no_comment = line.split("#", 1)[0].strip() + + if "==" not in line_no_comment: + continue + + name_with_extras, version = [p.strip() for p in line_no_comment.split("==", 1)] + if not name_with_extras or not version: + continue + + name_normalized = name_with_extras.split("[", 1)[0].strip().lower() + packages.append((raw_line, name_with_extras, name_normalized)) + Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Add explicit validation/guards at integration boundaries (e.g., parsing file inputs) to avoid mis-parsing unexpected formats and silently producing incorrect output.	Low
	✅ Make venv pip invocation portable Suggestion Impact: The commit addressed the portability concern by replacing the hard-coded "venv_dir/bin/pip" with a Windows vs Unix selection (Scripts/pip.exe vs bin/pip). It did not implement the specific recommendation to invoke pip via "python -m pip", but it follows the intent of making the pip invocation path cross-platform. code diff: - pip = venv_dir / "bin" / "pip" + # Cross-platform pip path: Scripts on Windows, bin on Unix + if platform.system() == "Windows": + pip = venv_dir / "Scripts" / "pip.exe" + else: + pip = venv_dir / "bin" / "pip" Avoid hard-coding the venv pip path (bin/pip) and instead invoke pip via the venv Python (python -m pip) to work across platforms and environments reliably. scripts/update_py_deps.py [45] -pip = venv_dir / "bin" / "pip" +venv_python = venv_dir / ("Scripts" if sys.platform.startswith("win") else "bin") / ( + "python.exe" if sys.platform.startswith("win") else "python" +) [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: Relevant best practice - Add explicit validation/availability guards at system/tool boundaries (interpreters, paths, subprocesses) and make calls robust/portable.	Low
General	Improve file reconstruction logic Refactor the logic for rebuilding requirements.txt to use a dictionary mapping package names to their updated lines. This makes the file reconstruction more robust and less dependent on line order. scripts/update_py_deps.py [95-104] # Rebuild file preserving non-package lines +updated_package_lines = { + p[2]: line for p, line in zip(packages, updated_lines) +} new_content = [] pkg_idx = 0 for line in current_lines: stripped = line.strip() if stripped and not stripped.startswith("#") and "==" in stripped: - new_content.append(updated_lines[pkg_idx]) + # This assumes the order and number of parsed packages match + _, _, name_normalized = packages[pkg_idx] + new_content.append(updated_package_lines[name_normalized]) pkg_idx += 1 else: new_content.append(line) Suggestion importance[1-10]: 5 __ Why: The suggestion proposes a refactoring that improves the robustness of the requirements.txt file reconstruction logic. While the current implementation works for well-formed files, the proposed change using a dictionary lookup is less prone to errors from unexpected formatting, thus improving maintainability.	Low
	Surface venv creation errors Remove capture_output=True from the subprocess.run call that creates the virtual environment. This will ensure that any error messages are displayed in the console, aiding in debugging. scripts/update_py_deps.py [39-43] subprocess.run( [sys.executable, "-m", "venv", str(venv_dir)], - check=True, - capture_output=True, + check=True ) Suggestion importance[1-10]: 4 __ Why: This is a useful suggestion for improving debuggability. By removing capture_output=True, any errors during the virtual environment creation will be printed to standard error, making it easier to diagnose failures in that step.	Low

[cgoldberg]

This needs some work

[cgoldberg]

oops.. closed it by accident. I'll review later today

[qodo-code-review]

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪	Supply-chain dependency risk Description: The new dependency update script creates a temporary venv and runs pip install against the live package index without hash-checking/pinning (e.g., pip install -q ...), which can introduce supply-chain risk (typosquatting, compromised upstream releases, or dependency confusion) if the requirements list is manipulated or an upstream package is compromised. update_py_deps.py [54-64] Referred Code # Install packages (with extras) to let pip resolve versions install_names = [p[1] for p in packages] # name_with_extras print(f"Installing {len(install_names)} packages...") result = subprocess.run( [str(pip), "install", "-q"] + install_names, capture_output=True, text=True, ) if result.returncode != 0: print(f"Error installing packages:\n{result.stderr}") return
Ticket Compliance
⚪	🎫 No ticket provided Create ticket/issue
Codebase Duplication Compliance
⚪	Codebase context is not defined Follow the guide to enable codebase context checks.
Custom Compliance
🟢	Generic: Comprehensive Audit Trails Objective: To create a detailed and reliable record of critical system actions for security analysis and compliance. Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Meaningful Naming and Self-Documenting Code Objective: Ensure all identifiers clearly express their purpose and intent, making code self-documenting Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Security-First Input Validation and Data Handling Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent vulnerabilities Status: Passed Learn more about managing compliance generic rules or creating your own custom rules
🔴	Generic: Robust Error Handling and Edge Case Management Objective: Ensure comprehensive error handling that provides meaningful context and graceful degradation Status: Missing failure exits: The script prints errors and returns without a non-zero exit code (and may raise uncaught CalledProcessError), which can make CI/reporting treat failures as success and lacks consistent contextual handling. Referred Code if not requirements_file.exists(): print(f"Error: {requirements_file} not found") return print(f"Checking {requirements_file}") # Parse current requirements preserving original format current_lines = requirements_file.read_text().strip().split("\n") packages = [] # (original_line, package_name_with_extras, package_name_normalized) for line in current_lines: line = line.strip() if line and not line.startswith("#") and "==" in line: name_with_extras, version = line.split("==", 1) # Normalize: remove extras for pip queries name_normalized = name_with_extras.split("[")[0].lower() packages.append((line, name_with_extras, name_normalized)) with tempfile.TemporaryDirectory() as tmpdir: venv_dir = Path(tmpdir) / "venv" # Create virtual environment ... (clipped 28 lines) Learn more about managing compliance generic rules or creating your own custom rules
⚪	Generic: Secure Error Handling Objective: To prevent the leakage of sensitive system information through error messages while providing sufficient detail for internal debugging. Status: Raw stderr printed: The script prints raw pip stderr directly to stdout, which may expose internal paths or environment details depending on the failure output and where logs are surfaced. Referred Code if result.returncode != 0: print(f"Error installing packages:\n{result.stderr}") return Learn more about managing compliance generic rules or creating your own custom rules
	Generic: Secure Logging Practices Objective: To ensure logs are useful for debugging and auditing without exposing sensitive information like PII, PHI, or cardholder data. Status: Unstructured console logs: The script emits unstructured console output (plain print) which may be harder to audit/parse in automation logs and should be validated against project logging expectations for build tooling. Referred Code print(f"Error: {requirements_file} not found") return print(f"Checking {requirements_file}") # Parse current requirements preserving original format current_lines = requirements_file.read_text().strip().split("\n") packages = [] # (original_line, package_name_with_extras, package_name_normalized) for line in current_lines: line = line.strip() if line and not line.startswith("#") and "==" in line: name_with_extras, version = line.split("==", 1) # Normalize: remove extras for pip queries name_normalized = name_with_extras.split("[")[0].lower() packages.append((line, name_with_extras, name_normalized)) with tempfile.TemporaryDirectory() as tmpdir: venv_dir = Path(tmpdir) / "venv" # Create virtual environment print("Creating temporary virtual environment...") ... (clipped 71 lines) Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend

🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
High-level	Simplify by using pip-tools Replace the custom update_py_deps.py script with the standard pip-tools library. This involves creating a requirements.in file for top-level dependencies and using pip-compile to generate the pinned requirements.txt. Examples: scripts/update_py_deps.py [1-112] #!/usr/bin/env python import json import subprocess import sys import tempfile from pathlib import Path # Updates py/requirements.txt with latest compatible versions using pip # Run with: bazel run //scripts:update_py_deps ... (clipped 102 lines) Solution Walkthrough: Before: # scripts/update_py_deps.py def main(): # Parse current requirements.txt current_lines = requirements_file.read_text().strip().split("\n") packages = parse_packages(current_lines) with tempfile.TemporaryDirectory() as tmpdir: # Create a temporary virtual environment subprocess.run([sys.executable, "-m", "venv", ...]) pip = venv_dir / "bin" / "pip" # Install packages to let pip resolve latest versions subprocess.run([str(pip), "install", ...]) # Get installed versions via `pip list` result = subprocess.run([str(pip), "list", "--format=json"], ...) installed = { ... } # Rebuild requirements.txt with new versions, preserving comments new_content = rebuild_file_with_updates(...) requirements_file.write_text("\n".join(new_content) + "\n") After: # py/requirements.in (new file) # List of top-level, unpinned dependencies trio trio-websocket urllib3[socks] # ... and other direct dependencies # Rakefile (updated task) desc 'Update Python dependencies' task :update do # Use pip-compile to generate requirements.txt sh 'pip-compile --upgrade py/requirements.in -o py/requirements.txt' Bazel.execute('run', [], '//py:requirements.update') @git.add('py/requirements.txt') @git.add('py/requirements_lock.txt') end # The 'scripts/update_py_deps.py' file would be deleted. Suggestion importance[1-10]: 9 __ Why: This is an excellent suggestion as the new update_py_deps.py script re-implements functionality already provided by standard tools, and using pip-tools would significantly simplify the code, reduce future maintenance, and align with community best practices.	High
Possible issue	✅ Handle Windows venv pip path Suggestion Impact: The commit adds OS detection and sets the pip path to venv_dir/Scripts/pip.exe on Windows and venv_dir/bin/pip otherwise, addressing the cross-platform venv pip path issue. code diff: +import platform import subprocess import sys import tempfile @@ -16,20 +17,28 @@ if not requirements_file.exists(): print(f"Error: {requirements_file} not found") - return + sys.exit(1) print(f"Checking {requirements_file}") # Parse current requirements preserving original format - current_lines = requirements_file.read_text().strip().split("\n") + current_lines = requirements_file.read_text().split("\n") + # Remove trailing empty line from final newline if present + if current_lines and current_lines[-1] == "": + current_lines = current_lines[:-1] + packages = [] # (original_line, package_name_with_extras, package_name_normalized) for line in current_lines: - line = line.strip() - if line and not line.startswith("#") and "==" in line: - name_with_extras, version = line.split("==", 1) + stripped = line.strip() + if stripped and not stripped.startswith("#") and "==" in stripped: + name_with_extras, version = stripped.split("==", 1) # Normalize: remove extras for pip queries name_normalized = name_with_extras.split("[")[0].lower() packages.append((line, name_with_extras, name_normalized)) + + if not packages: + print("No packages found in requirements file") + sys.exit(1) with tempfile.TemporaryDirectory() as tmpdir: venv_dir = Path(tmpdir) / "venv" @@ -42,7 +51,11 @@ capture_output=True, ) - pip = venv_dir / "bin" / "pip" + # Cross-platform pip path: Scripts on Windows, bin on Unix + if platform.system() == "Windows": + pip = venv_dir / "Scripts" / "pip.exe" + else: + pip = venv_dir / "bin" / "pip" Ensure cross-platform compatibility by detecting the operating system to correctly locate the pip executable within the virtual environment, which is in bin on Unix and Scripts on Windows. scripts/update_py_deps.py [45] -pip = venv_dir / "bin" / "pip" +import os +pip_dir = "Scripts" if os.name == "nt" else "bin" +pip_name = "pip.exe" if os.name == "nt" else "pip" +pip = venv_dir / pip_dir / pip_name [Suggestion processed] Suggestion importance[1-10]: 7 __ Why: This suggestion correctly identifies and fixes a cross-platform compatibility issue by dynamically setting the pip executable path based on the operating system, making the script more robust.	Medium
General	✅ Use non-zero exit on missing file Suggestion Impact: The script was updated to call sys.exit(1) instead of returning when the requirements file does not exist, making the failure detectable via a non-zero exit code. It did not adopt the suggested stderr print change. code diff: if not requirements_file.exists(): print(f"Error: {requirements_file} not found") - return + sys.exit(1) Modify the script to exit with a non-zero status code if the requirements.txt file is not found, ensuring that CI pipelines can detect this failure. scripts/update_py_deps.py [17-19] if not requirements_file.exists(): - print(f"Error: {requirements_file} not found") - return + print(f"Error: {requirements_file} not found", file=sys.stderr) + sys.exit(1) [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: This suggestion correctly improves error handling by ensuring the script exits with a non-zero status code on a critical failure, which allows CI/CD pipelines to properly detect and report the error.	Low
	✅ Remove confusing instructional message Suggestion Impact: The commit removed the confusing/redundant print statement ("\nNow run: bazel run //py:requirements.update") from the script output. code diff: requirements_file.write_text("\n".join(new_content) + "\n") print(f"\nUpdated {len(updates)} package(s)") - print("\nNow run: bazel run //py:requirements.update") Remove the redundant instructional message print("\nNow run: bazel run //py:requirements.update"), as the calling Rakefile task already executes this command. scripts/update_py_deps.py [108] -print("\nNow run: bazel run //py:requirements.update") +# This line should be removed. [Suggestion processed] Suggestion importance[1-10]: 5 __ Why: The suggestion correctly points out that the instructional message is redundant and potentially confusing since the calling Rakefile task already performs the action, improving the user experience.	Low
Learned best practice	✅ Preserve original file formatting Suggestion Impact: The commit removed the full-file .strip() when reading requirements and introduced a per-line `stripped = line.strip()` used for parsing (instead of mutating `line`). It also added handling for the trailing empty line caused by a final newline. This aligns with the intent to reduce unintended formatting changes, though it did not fully switch to `splitlines()` nor store the stripped line in `packages` as suggested. code diff: # Parse current requirements preserving original format - current_lines = requirements_file.read_text().strip().split("\n") + current_lines = requirements_file.read_text().split("\n") + # Remove trailing empty line from final newline if present + if current_lines and current_lines[-1] == "": + current_lines = current_lines[:-1] + packages = [] # (original_line, package_name_with_extras, package_name_normalized) for line in current_lines: - line = line.strip() - if line and not line.startswith("#") and "==" in line: - name_with_extras, version = line.split("==", 1) + stripped = line.strip() + if stripped and not stripped.startswith("#") and "==" in stripped: + name_with_extras, version = stripped.split("==", 1) # Normalize: remove extras for pip queries name_normalized = name_with_extras.split("[")[0].lower() packages.append((line, name_with_extras, name_normalized)) Avoid strip() on the whole file and per-line, since it can unintentionally remove leading/trailing blank lines and indentation you later try to preserve; keep the raw line and only use a stripped copy for parsing. scripts/update_py_deps.py [24-32] -current_lines = requirements_file.read_text().strip().split("\n") +current_lines = requirements_file.read_text().splitlines() packages = [] # (original_line, package_name_with_extras, package_name_normalized) for line in current_lines: - line = line.strip() - if line and not line.startswith("#") and "==" in line: - name_with_extras, version = line.split("==", 1) + stripped = line.strip() + if stripped and not stripped.startswith("#") and "==" in stripped: + name_with_extras, _version = stripped.split("==", 1) # Normalize: remove extras for pip queries name_normalized = name_with_extras.split("[")[0].lower() - packages.append((line, name_with_extras, name_normalized)) + packages.append((stripped, name_with_extras, name_normalized)) [Suggestion processed] Suggestion importance[1-10]: 6 __ Why: Relevant best practice - Reduce hidden side effects by avoiding unintended input normalization/mutation when reading external inputs.	Low
More

[titusfortner]

@cgoldberg There aren't any changes when I run this, so it isn't needed for the current release, so not a rush. Much better that we ensure it does the same thing your shell script does as well as it did.

[Copilot]

Pull request overview

This PR automates Python dependency updates as part of the pre-release workflow, bringing Python dependency management in line with how Java, Ruby, and Rust handle their dependencies.

Changes:

• Replaces shell script with cross-platform Python implementation for updating py/requirements.txt
• Integrates Python dependency updates into the pre-release CI workflow
• Adds Bazel targets and Rake tasks for streamlined dependency management

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
scripts/update_py_deps.py	New Python script to update dependencies with latest compatible versions while preserving extras and formatting
scripts/update_py_dependencies.sh	Removed legacy shell script implementation
scripts/BUILD.bazel	Added Bazel target for the new update_py_deps script
Rakefile	Added py:update and py:pin tasks with proper git staging
.github/workflows/pre-release.yml	Integrated py:update into pre-release workflow alongside Java updates

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

[cgoldberg]

LGTM 👍 I just tried this and it seems to work correctly.

[cgoldberg]

there are some merge conflicts in the Rakefile that need to be fixed

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.