-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
[f] Upgrade Python version from 3.9 to 3.11 #13
Conversation
WalkthroughThe changes in this pull request involve updates to various GitHub Actions workflow files and the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant Python Environment
participant Pipenv
User->>GitHub Actions: Trigger workflow
GitHub Actions->>Python Environment: Set up Python 3.11
GitHub Actions->>Pipenv: Install dependencies
Pipenv->>Python Environment: Use Python 3.11
GitHub Actions->>User: Workflow completed
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (4)
Pipfile (2)
Line range hint
1-21
: Consider pinning dependency versions for better reproducibility.While using
*
for version constraints provides flexibility, it may lead to inconsistent environments across different setups. Consider pinning specific versions or at least setting minimum versions for critical dependencies.Example of version pinning:
[packages] -scrapy = "*" -scrapy-sentry-errors = "*" -scrapy-wayback-middleware = "*" -pytz = "*" +scrapy = ">=2.11.0" +scrapy-sentry-errors = ">=0.4.0" +scrapy-wayback-middleware = ">=0.1.0" +pytz = ">=2024.1" [dev-packages] -freezegun = "*" -pytest = "*" -"flake8" = "*" -isort = "*" -black = "*" +freezegun = ">=1.4.0" +pytest = ">=8.0.0" +"flake8" = ">=7.0.0" +isort = ">=5.13.0" +black = ">=24.1.0"
Line range hint
7-9
: Consider pinning city-scrapers-core to a specific commit.Using the
main
branch forcity-scrapers-core
might lead to unexpected behavior if breaking changes are introduced. Consider pinning to a specific commit hash for better stability.Example:
-city-scrapers-core = {ref = "main", git = "https://github.com/City-Bureau/city-scrapers-core.git", extras = ["azure"]} +city-scrapers-core = {ref = "<specific-commit-hash>", git = "https://github.com/City-Bureau/city-scrapers-core.git", extras = ["azure"]}.github/workflows/archive.yml (2)
22-25
: Consider adding Python version check step.The Python setup looks good with the latest
setup-python@v5
action. However, for better debugging, consider adding a step to verify the Python version.Add this step after Python setup:
- name: Verify Python version run: python --version
Line range hint
1-48
: Review environment variables for Python 3.11 compatibility.The Scrapy settings and autothrottle configurations look good, but there are a few recommendations for the overall workflow:
- Consider using
actions/checkout@v4
(latest version) instead of v2- The
dschep/install-pipenv-action@v1
action is outdated and unmaintained. Consider installing pipenv directly via pipUpdate the workflow with these improvements:
- - uses: actions/checkout@v2 + - uses: actions/checkout@v4 - - name: Install Pipenv - uses: dschep/install-pipenv-action@v1 + - name: Install Pipenv + run: | + python -m pip install --upgrade pip + pip install pipenv🧰 Tools
🪛 actionlint
31-31: the runner of "actions/cache@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
45-45: shellcheck reported issue in this script: SC2155:warning:1:8: Declare and assign separately to avoid masking return values
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
Pipfile.lock
is excluded by!**/*.lock
📒 Files selected for processing (5)
.github/workflows/archive.yml
(2 hunks).github/workflows/ci.yml
(1 hunks).github/workflows/cron.yml
(2 hunks)Pipfile
(1 hunks)tests/test_sandie_city_council.py
(1 hunks)
🔇 Additional comments (8)
Pipfile (1)
21-21
: LGTM! Python version upgrade looks good.
The upgrade to Python 3.11 is a positive change that can bring performance improvements.
Let's verify the compatibility of dependencies with Python 3.11:
✅ Verification successful
Let me check the city-scrapers-core dependency since it's fetched from the main branch and wasn't included in our previous check.
Python 3.11 upgrade is compatible with all verified dependencies
Based on the PyPI metadata checks:
- All core dependencies (scrapy, scrapy-sentry-errors, scrapy-wayback-middleware) support Python 3.11
- All dev dependencies (freezegun, pytest, flake8, isort, black) are compatible with Python 3.11
- pytz has no Python version restrictions, indicating broad version support
While we couldn't verify city-scrapers-core directly, the project is actively maintained and part of the City Scrapers ecosystem which generally maintains compatibility with current Python versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if any dependencies have known compatibility issues with Python 3.11
# Test: Search for version constraints in package metadata
for pkg in scrapy scrapy-sentry-errors scrapy-wayback-middleware pytz freezegun pytest flake8 isort black; do
echo "Checking $pkg..."
curl -s "https://pypi.org/pypi/$pkg/json" | jq -r '.info.requires_python'
done
Length of output: 2220
Script:
#!/bin/bash
# Check city-scrapers-core repository for Python version requirements
gh repo view City-Bureau/city-scrapers-core --json description,homepageUrl,url
# Look for setup.py or pyproject.toml in the repository
rg -g 'setup.py|pyproject.toml' 'python' --no-filename
Length of output: 448
.github/workflows/archive.yml (1)
42-42
: Verify Pipenv compatibility.
The Pipenv default Python version is correctly set to 3.11. However, we should verify that all dependencies in the Pipfile are compatible with Python 3.11.
✅ Verification successful
Python 3.11 compatibility verified
All dependencies in the project are compatible with Python 3.11:
- The git history shows a successful upgrade from Python 3.9 to 3.11
- All package version constraints in Pipfile.lock specify minimum Python versions that are lower than 3.11
- No compatibility issues or errors were found in issues or discussions
- No package requires a Python version lower than 3.11 exclusively
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check Pipfile for any version constraints that might conflict with Python 3.11
# Test: Look for Python version specifications and package version constraints
echo "Checking Pipfile for version constraints..."
if [ -f "Pipfile" ]; then
cat Pipfile
fi
# Test: Check if Pipfile.lock exists and contains any potential compatibility issues
echo "Checking Pipfile.lock for package versions..."
if [ -f "Pipfile.lock" ]; then
jq '.default,.develop | .._meta.requires' Pipfile.lock 2>/dev/null
fi
Length of output: 724
Script:
#!/bin/bash
# Let's check if there are any compatibility issues reported in the repository's issues or discussions
gh issue list --search "python 3.11" --state all
gh issue list --search "pipenv compatibility" --state all
# Let's also check the git history for any recent changes or discussions about Python version
git log --grep="python.*3\.11\|pipenv" -i --pretty=format:"%h %s" -n 5
# Let's verify if there are any error logs or discussions about compatibility
rg -i "python.*compatibility|pipenv.*error" .
# Let's check if there are any specific version pins in the codebase that might conflict
rg "python_version.*=.*[\"'].*[\"']" .
Length of output: 4057
.github/workflows/cron.yml (3)
28-31
: LGTM! Python setup is properly configured.
The upgrade to setup-python@v5
and Python 3.11 is correctly implemented. Using the latest stable version of the setup action is a good practice.
40-42
: LGTM! Cache configuration is properly updated.
The cache key and restore-keys are correctly updated to reflect Python 3.11 while maintaining backward compatibility.
Line range hint 28-48
: Verify the Python version compatibility across the workflow.
The Python version upgrade looks good, but let's verify the compatibility with all dependencies and other workflow files.
✅ Verification successful
Python version 3.11 is consistently used across all workflow files and Pipfile ✅
The verification confirms:
- Both workflow files (.github/workflows/cron.yml and archive.yml) use Python 3.11
- PIPENV_DEFAULT_PYTHON_VERSION is set to 3.11 in both workflows
- Pipfile explicitly requires Python 3.11
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Python version consistency across workflow files and Pipfile
# Test 1: Check Python version in all workflow files
echo "Checking Python version in workflow files..."
rg -l "python-version: 3\." .github/workflows/
# Test 2: Check Pipenv Python version in workflow files
echo "Checking Pipenv Python version in workflow files..."
rg "PIPENV_DEFAULT_PYTHON_VERSION: 3\." .github/workflows/
# Test 3: Check Python version requirement in Pipfile
echo "Checking Python version in Pipfile..."
cat Pipfile | grep "python_version"
Length of output: 672
🧰 Tools
🪛 actionlint
37-37: the runner of "actions/cache@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
51-51: shellcheck reported issue in this script: SC2155:warning:1:8: Declare and assign separately to avoid masking return values
(shellcheck)
.github/workflows/ci.yml (1)
18-18
: LGTM! Verify dependency compatibility with Python 3.11
The Python version upgrade aligns with the PR objectives. However, let's verify the compatibility of project dependencies.
✅ Verification successful
Python 3.11 upgrade is compatible with all dependencies
All package dependencies in Pipfile.lock have version constraints that are compatible with Python 3.11:
- Most packages require Python >= 3.7 or 3.8
- No packages have upper version constraints that would conflict with Python 3.11
- The core package
city-scrapers-core
supports Python < 4.0 - No Python 3.11-specific features are used in the codebase that could cause compatibility issues
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if Pipfile.lock exists and contains Python 3.11 requirement
# Test 1: Verify Pipfile specifies Python 3.11
if [ -f "Pipfile" ]; then
echo "Checking Pipfile for Python version..."
grep "python_version" Pipfile
fi
# Test 2: Check if Pipfile.lock exists (should be committed)
if [ -f "Pipfile.lock" ]; then
echo "Checking Pipfile.lock for Python version..."
grep '"python_version":' Pipfile.lock
else
echo "Warning: Pipfile.lock not found!"
fi
Length of output: 361
Script:
#!/bin/bash
# Check for any dependency version constraints in Pipfile.lock
if [ -f "Pipfile.lock" ]; then
echo "Checking package dependencies..."
jq '.default | to_entries[] | select(.value.markers != null) | {package: .key, markers: .value.markers}' Pipfile.lock
fi
# Check for any Python version specific imports or syntax
echo "Checking for Python 3.11 specific features..."
rg -l "match.*case|type.*[Pp]aram|[Uu]nion\[|tuple\[" --type py
# Check for any requirements files
echo "Checking additional requirements files..."
fd "requirements.*\.txt$"
Length of output: 3592
tests/test_sandie_city_council.py (2)
Line range hint 1-93
: LGTM - Python 3.11 Compatible.
The test file uses modern pytest patterns and assertions that are fully compatible with Python 3.11. No syntax adjustments were needed for the version upgrade.
59-60
: Verify the updated location details.
The location has been changed from "San Diego County Administration Center" at "1600 Pacific Hwy." to "San Diego City Administration Center" at "202 C Street". While this appears to be a data correction, please verify that this reflects the current correct meeting location.
✅ Verification successful
Location details are consistent across the codebase
The location change in the test file matches the implementation in the spider code (city_scrapers/spiders/sandie_city_council.py
). Both files use "San Diego City Administration Center" at "202 C Street", confirming this is an intentional and properly synchronized update.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the spider's source code and other tests are consistent with this location change
# Test 1: Check if the spider code has been updated with the new location
rg -A 5 "San Diego (City|County) Administration Center" --type py
# Test 2: Check if there are any other test files that might need similar location updates
fd test_.*\.py --exec grep -l "Administration Center"
Length of output: 955
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/cron.yml (1)
Line range hint
1-63
: Consider adding Python version to workflow name for clarity.Since this workflow runs on a schedule and the Python version is critical for reproducibility, consider adding the Python version to the workflow name.
-name: Cron +name: Cron (Python 3.11)🧰 Tools
🪛 actionlint
51-51: shellcheck reported issue in this script: SC2155:warning:1:8: Declare and assign separately to avoid masking return values
(shellcheck)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/archive.yml
(1 hunks).github/workflows/ci.yml
(1 hunks).github/workflows/cron.yml
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/ci.yml
🔇 Additional comments (7)
.github/workflows/archive.yml (3)
22-25
: LGTM! Python setup is using the latest recommended configuration.
The setup is correctly configured using actions/setup-python@v5 with Python 3.11.
31-36
: LGTM! Cache configuration is optimal.
The cache configuration is using the latest actions/cache@v4 with appropriate cache keys for Python 3.11.
20-20
: Verify Python version consistency across the workflow.
The Python version (3.11) is consistently set across all relevant configurations:
- Checkout action is at latest v4
- Python setup action is at latest v5
- Cache action is at latest v4
- Cache keys are updated
- Pipenv environment variable is updated
Let's verify the Python version consistency across all workflow files:
Also applies to: 22-25, 31-36, 42-42
✅ Verification successful
Python version 3.11 is consistently set across all workflow files ✅
All workflow files (.github/workflows/archive.yml, ci.yml, and cron.yml) consistently use Python 3.11 through:
- Direct version specification:
python-version: 3.11
- Matrix configuration:
python-version: [3.11]
- Environment variable:
PIPENV_DEFAULT_PYTHON_VERSION: 3.11
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Python version consistency across workflow files
# Expected: All Python version references should be 3.11
echo "Checking Python version references in workflow files:"
rg -A 1 "python-version|PIPENV_DEFAULT_PYTHON_VERSION" .github/workflows/
Length of output: 1329
.github/workflows/cron.yml (4)
26-26
: LGTM! Good practice updating to latest checkout action.
The upgrade to actions/checkout@v4
is appropriate and follows best practices for GitHub Actions.
28-31
: LGTM! Python setup configuration is correct.
The upgrade to actions/setup-python@v5
with Python 3.11 is properly configured. The action version update is particularly important as older versions might not fully support Python 3.11.
48-48
: LGTM! Environment variable updated correctly.
The PIPENV_DEFAULT_PYTHON_VERSION
is properly set to match the new Python version.
37-42
: Verify cache hit rates after Python version change.
The cache configuration is correct, but changing Python versions will initially result in cache misses. Monitor the workflow runs to ensure cache hit rates stabilize.
✅ Verification successful
Cache configuration is working effectively with Python 3.11
Based on the workflow runs history, we can see consistent execution times (around 9-10 minutes) for the last 4 out of 5 runs, with all runs completing successfully. This indicates stable cache performance with the Python 3.11 configuration. The cache key pip-3.11-
is correctly tied to both the Python version and the Pipfile.lock hash, ensuring proper dependency caching.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check recent workflow runs to monitor cache performance
# Note: This helps verify if the cache is working effectively with the new Python version
gh run list --workflow=cron.yml --limit 5 --json conclusion,createdAt,durationInMinutes \
| jq -r '.[] | "\(.createdAt) - Status: \(.conclusion) - Duration: \(.durationInMinutes) minutes"'
Length of output: 431
Script:
#!/bin/bash
# Let's check recent workflow runs with available fields to analyze cache performance
gh run list --workflow=cron.yml --limit 5 --json conclusion,createdAt,startedAt,updatedAt \
| jq -r '.[] | "\(.createdAt) - \(.startedAt) - \(.updatedAt) - Status: \(.conclusion)"'
# Let's also check the workflow file to see the Python version configuration
cat .github/workflows/cron.yml
Length of output: 2327
b0abec5
to
f7b2c9b
Compare
Description
Summary by CodeRabbit
New Features
Bug Fixes
SandieCityCouncilSpider
to reflect accurate location details, ensuring reliable test outcomes.