-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Use python 3.12 on Netlify and CI #4375
Conversation
WalkthroughThe pull request introduces updates to two configuration files: Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GitHub Actions
participant Netlify
User->>GitHub Actions: Push to current or next branch
GitHub Actions->>GitHub Actions: Trigger linting workflow
GitHub Actions->>GitHub Actions: Set up Python 3.12
GitHub Actions->>Netlify: Build with PYTHON_VERSION 3.12
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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: 1
🧹 Outside diff range and nitpick comments (1)
netlify.toml (1)
Python version should be explicitly set in all Netlify contexts
The verification confirms that:
- Python 3.12 is explicitly used in:
- GitHub Actions workflow (lint.yml)
- Default Netlify build context
- No Python version constraints exist in requirements.txt or requirements_test.txt
- Beta, next, and production contexts in netlify.toml lack Python version specification
To maintain consistency across all deployment environments and prevent potential issues from different Python versions, the original review comment's suggestion to add PYTHON_VERSION to all contexts is valid.
🔗 Analysis chain
Line range hint
9-17
: Consider adding PYTHON_VERSION to other contexts.The Python version is only set in the default build context but not in beta, next, or production contexts. This might lead to inconsistent Python versions across different deployment environments.
Consider explicitly setting
PYTHON_VERSION = "3.12"
in all contexts to ensure consistent behavior across environments:[context.beta] - environment = { BASE_URL = "https://beta.esphome.io" } + environment = { BASE_URL = "https://beta.esphome.io", PYTHON_VERSION = "3.12" } [context.next] - environment = { BASE_URL = "https://next.esphome.io" } + environment = { BASE_URL = "https://next.esphome.io", PYTHON_VERSION = "3.12" } [context.production] - environment = { BASE_URL = "https://esphome.io", PRODUCTION = "YES" } + environment = { BASE_URL = "https://esphome.io", PRODUCTION = "YES", PYTHON_VERSION = "3.12" }
Let me check the content of the workflow file and the complete netlify.toml to make a better assessment.
Let me check if there are any Python version requirements in the requirements files to ensure we have a complete picture.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if there are any Python version constraints in other configuration files # that might conflict with this change. # Look for Python version specifications in requirements files echo "Checking requirements files for Python version constraints:" rg -l "python_version|Python>=|Python>|Python==|Python<=|Python<" --type yaml --type txt # Look for Python version specifications in workflow files echo "Checking workflow files for Python version specifications:" rg -l "python-version:" .github/workflows/Length of output: 416
Script:
#!/bin/bash # Check the content of the workflow file for Python version echo "Content of .github/workflows/lint.yml:" cat .github/workflows/lint.yml # Check the complete netlify.toml echo -e "\nContent of netlify.toml:" cat netlify.tomlLength of output: 1587
Script:
#!/bin/bash # Check requirements files for any Python version constraints echo "Content of requirements.txt:" cat requirements.txt echo -e "\nContent of requirements_test.txt:" cat requirements_test.txtLength of output: 323
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- .github/workflows/lint.yml (2 hunks)
- netlify.toml (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
netlify.toml (1)
Pattern
**
: - Do not generate or add any sequence diagrams
🔇 Additional comments (2)
netlify.toml (1)
4-7
: LGTM! Python version configuration looks correct.The addition of
PYTHON_VERSION = "3.12"
aligns with Netlify's configuration for Python versions..github/workflows/lint.yml (1)
14-16
: LGTM! Well-implemented concurrency control.The concurrency configuration is a good addition that will help optimize CI resources by canceling outdated runs when new changes are pushed.
- name: Set up Python 3.12 | ||
uses: actions/setup-python@v5 | ||
with: | ||
python-version: 3.8 | ||
python-version: 3.12 |
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.
💡 Codebase verification
Potential compatibility issues with Python 3.12 detected
Based on the PyPI metadata:
- sphinx 7.1.2 doesn't explicitly support Python 3.12 (supports up to 3.11)
- sphinx-autobuild 2021.3.14 is quite outdated (supports only up to 3.8)
- sphinx-tabs 3.4.5 doesn't explicitly support Python 3.12 (supports up to 3.10)
- sphinx-toolbox 3.8.0 and Pillow explicitly support Python 3.12
Recommended actions:
- Update sphinx-autobuild to the latest version
- Verify if newer versions of sphinx and sphinx-tabs are available with Python 3.12 support
🔗 Analysis chain
Verify dependency compatibility with Python 3.12.
The Python version update looks good and aligns with the PR objectives. However, let's verify that all dependencies in requirements.txt and requirements_test.txt are compatible with Python 3.12.
Let me verify the compatibility of these dependencies with Python 3.12 by checking their PyPI metadata.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any potential compatibility issues with Python 3.12 in requirements files
echo "Checking requirements files for potential compatibility issues..."
# Display the content of both requirements files
echo "=== requirements.txt ==="
cat requirements.txt
echo -e "\n=== requirements_test.txt ==="
cat requirements_test.txt
Length of output: 448
Script:
#!/bin/bash
# Check PyPI metadata for Python version compatibility using gh (GitHub CLI)
# focusing on the main dependencies
echo "=== Checking Sphinx compatibility ==="
curl -s "https://pypi.org/pypi/sphinx/7.1.2/json" | jq -r '.info.requires_python, .info.classifiers[] | select(contains("Programming Language :: Python"))'
echo -e "\n=== Checking sphinx-autobuild compatibility ==="
curl -s "https://pypi.org/pypi/sphinx-autobuild/2021.3.14/json" | jq -r '.info.requires_python, .info.classifiers[] | select(contains("Programming Language :: Python"))'
echo -e "\n=== Checking sphinx-tabs compatibility ==="
curl -s "https://pypi.org/pypi/sphinx-tabs/3.4.5/json" | jq -r '.info.requires_python, .info.classifiers[] | select(contains("Programming Language :: Python"))'
echo -e "\n=== Checking sphinx-toolbox compatibility ==="
curl -s "https://pypi.org/pypi/sphinx-toolbox/3.8.0/json" | jq -r '.info.requires_python, .info.classifiers[] | select(contains("Programming Language :: Python"))'
echo -e "\n=== Checking Pillow compatibility ==="
curl -s "https://pypi.org/pypi/Pillow/json" | jq -r '.info.requires_python, .info.classifiers[] | select(contains("Programming Language :: Python"))'
Length of output: 2820
✅ Deploy Preview for esphome ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Description:
Finally Netlify supports setting the python version to something higher than 3.8!
https://www.netlify.com/blog/announcing-configurable-python-versions-in-netlify-builds/
Related issue (if applicable): fixes
Pull request in esphome with YAML changes (if applicable): esphome/esphome#
Checklist:
I am merging into
next
because this is new documentation that has a matching pull-request in esphome as linked above.or
I am merging into
current
because this is a fix, change and/or adjustment in the current documentation and is not for a new component or feature.Link added in
/index.rst
when creating new documents for new components or cookbook.