Support Python 3.12 and above versions #17
Conversation
|
Warning Rate limit exceeded@allisonwang-db has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 4 minutes and 8 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
WalkthroughA new GitHub Actions workflow for continuous integration was added, enabling automated testing across multiple Python versions. The Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub
participant CI Workflow
participant Runner
participant Poetry
participant TestSuite
GitHub->>CI Workflow: Push or PR to main/master
CI Workflow->>Runner: Start job for each Python version
Runner->>Runner: Checkout code
Runner->>Runner: Set up Python
Runner->>Poetry: Install Poetry
Runner->>Poetry: Configure venvs in project
Runner->>Runner: Cache .venv based on lock hash
alt Cache miss
Runner->>Poetry: Install dependencies
end
Runner->>Poetry: Install project
Runner->>TestSuite: Run tests (verbose)
Runner->>TestSuite: Run tests with coverage
Estimated code review effort2 (~15 minutes) Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
allisonwang-db
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
.gitignore (1)
161-167: Directories ignored look correct—consider grouping with related LLM tooling entriesThe new
.claude/,claude_cache/, and.gemini/patterns are fine, but they are appended far from the existing “IDE / tooling” block (PyCharm, Rope, etc.). Moving them next to similar IDE- or tool-specific ignores keeps the file logically grouped and easier to scan..github/workflows/ci.yml (2)
50-55: Tests run twice—drop the first invocation to save CI minutesThe standalone “Run tests” step (lines 50-52) duplicates the coverage run. Remove it and keep only the coverage-enabled command.
- - name: Run tests - run: poetry run pytest tests/ -v
1-56: Clean up trailing whitespace & ensure newline at EOFYAML-Lint flags several trailing-space offences and a missing final newline. They don’t break the workflow but failing the linter will block merges if enforced.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
.github/workflows/ci.yml(1 hunks).gitignore(1 hunks)pyproject.toml(2 hunks)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 56-56: no new line character at the end of file
(new-line-at-end-of-file)
[error] 56-56: trailing spaces
(trailing-spaces)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 30-30: trailing spaces
(trailing-spaces)
[error] 35-35: trailing spaces
(trailing-spaces)
[error] 42-42: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 52-52: trailing spaces
(trailing-spaces)
[error] 56-56: no new line character at the end of file
(new-line-at-end-of-file)
[error] 56-56: trailing spaces
(trailing-spaces)
🔇 Additional comments (3)
pyproject.toml (1)
36-36: Confirmed: pyspark 4.0.0 is available on PyPI
The PyPI metadata shows that version 4.0.0 exists, so locking to this release will not breakpoetry install..github/workflows/ci.yml (2)
15-16:3.13may be unsupported byactions/setup-pythonright nowIf GitHub Actions does not yet ship a stable 3.13 image, the job will fail. Consider pinning to
3.13-devor removing it until a GA release lands, then let the CI itself reveal readiness.
26-35: Poetry install split—but the second call makes the first redundantRunning
poetry install --no-rootfollowed by a fullpoetry installrepopulates the venv and defeats caching. Merge them into one step:- - name: Install dependencies - if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true' - run: poetry install --no-interaction --no-root - - - name: Install project - run: poetry install --no-interaction + - name: Install dependencies & project + if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true' + run: poetry install --no-interactionLikely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
47-52: Duplicate test execution – run once with coverageThe first pytest invocation (lines 47-48) is immediately repeated with coverage (lines 50-52).
Executing the suite twice inflates CI time and may hit external-resource rate limits.- - name: Run tests - run: poetry run pytest tests/ -v - - - name: Run tests with coverage - run: | - poetry run pytest tests/ --cov=pyspark_datasources --cov-report=xml --cov-report=term-missing + - name: Run tests with coverage + run: | + poetry run pytest tests/ --cov=pyspark_datasources --cov-report=xml --cov-report=term-missing -v
13-53: Cleanup trailing whitespace & add newline EOFYAML-lint flags stray spaces on several lines (16, 20, 25, 32, 39, 43, 46, 49) and the file lacks a final newline.
Cosmetic, but keeping the repo lint-clean avoids noisy CI in the future.- matrix: - python-version: ['3.9', '3.10', '3.11', '3.12', '3.13']␠ + matrix: + python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] ... - poetry run pytest tests/ --cov=pyspark_datasources --cov-report=xml --cov-report=term-missing␠ + poetry run pytest tests/ --cov=pyspark_datasources --cov-report=xml --cov-report=term-missing +
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
poetry.lockis excluded by!**/*.lock
📒 Files selected for processing (2)
.github/workflows/ci.yml(1 hunks)pyproject.toml(2 hunks)
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 39-39: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 53-53: no new line character at the end of file
(new-line-at-end-of-file)
[error] 53-53: trailing spaces
(trailing-spaces)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/ci.yml
[error] 16-16: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
[error] 25-25: trailing spaces
(trailing-spaces)
[error] 32-32: trailing spaces
(trailing-spaces)
[error] 39-39: trailing spaces
(trailing-spaces)
[error] 43-43: trailing spaces
(trailing-spaces)
[error] 46-46: trailing spaces
(trailing-spaces)
[error] 49-49: trailing spaces
(trailing-spaces)
[error] 53-53: no new line character at the end of file
(new-line-at-end-of-file)
[error] 53-53: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: test (3.12)
- GitHub Check: test (3.13)
- GitHub Check: test (3.10)
- GitHub Check: test (3.9)
- GitHub Check: test (3.11)
🔇 Additional comments (1)
.github/workflows/ci.yml (1)
15-15: Verify 3.13 image availability before merging
actions/setup-pythononly provides release streams that are published on python.org. At the moment Python 3.13 is still pre-release, so the plain string'3.13'will cause the job to fail as soon as the matrix expands to that runner.
Either switch to3.13-dev(nightly) and passprerelease: true, or drop it until the final tarball is out.- python-version: ['3.9', '3.10', '3.11', '3.12', '3.13'] + python-version: ['3.9', '3.10', '3.11', '3.12', '3.13-dev'] + + # If you keep the dev stream, instruct setup-python to allow prereleases + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + check-latest: true # picks the freshest dev tag
| - name: Install dependencies | ||
| if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true' | ||
| run: poetry install --no-interaction --no-root --extras "all" | ||
|
|
||
| - name: Install project | ||
| run: poetry install --no-interaction --extras "all" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Redundant poetry install runs – consolidate for speed
poetry install --no-root (line 42) installs all deps except the project itself, and the very next step (line 45) installs everything again including the project.
Running the resolver twice roughly doubles setup time. Merge them into a single call and keep the if guard to leverage the cache:
- if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
- run: poetry install --no-interaction --no-root --extras "all"
+ if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
+ run: poetry install --no-interaction --extras "all"Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 43-43: trailing spaces
(trailing-spaces)
🤖 Prompt for AI Agents
In .github/workflows/ci.yml around lines 40 to 45, there are two consecutive
poetry install commands causing redundant dependency resolution and slowing down
the CI. Remove the second poetry install step and modify the first step to run
unconditionally but only if the cache is missed, combining the flags to install
all dependencies including the project in one call. This consolidation will
speed up the setup by avoiding duplicate installs.
Remove Python version upper bound and support Python 3.12 and above versions.
Summary by CodeRabbit