Skip to content

Conversation

@shanemcd
Copy link

@shanemcd shanemcd commented Jan 14, 2026

Description

Use sqlalchemy[asyncio]>=2.0.42 instead of sqlalchemy>=2.0.42 to ensure greenlet is installed on all platforms.

SQLAlchemy's base dependencies specify greenlet with platform markers that exclude s390x. Since we use async SQLAlchemy features (via llama-stack), we need greenlet at runtime on all platforms.

The [asyncio] extra includes greenlet unconditionally, fixing s390x builds that were failing with "No module named 'greenlet'".

Type of change

  • Bug fix

Tools used to create PR

  • Assisted-by: Claude Code
  • Generated by: Claude Opus 4.5

Related Tickets & Documents

  • Related Issue # N/A
  • Closes # N/A

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Verify s390x container build succeeds with greenlet properly installed via sqlalchemy[asyncio] dependency.

Summary by CodeRabbit

  • Chores
    • Enabled asynchronous SQLAlchemy support to allow async database operations and improve compatibility with asyncio-based workflows.

✏️ Tip: You can customize this high-level summary in your review settings.

@openshift-ci
Copy link

openshift-ci bot commented Jan 14, 2026

Hi @shanemcd. Thanks for your PR.

I'm waiting for a lightspeed-core member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 14, 2026

Walkthrough

The project's SQLAlchemy dependency in pyproject.toml was changed from sqlalchemy>=2.0.42 to sqlalchemy[asyncio]>=2.0.42, enabling SQLAlchemy's asyncio extras.

Changes

Cohort / File(s) Summary
Dependency configuration
pyproject.toml
Replace sqlalchemy>=2.0.42 with sqlalchemy[asyncio]>=2.0.42 in [project.dependencies].

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: updating SQLAlchemy to use the [asyncio] extra to ensure greenlet availability across platforms.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.



📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4317ae8 and a1917cc.

📒 Files selected for processing (1)
  • pyproject.toml
🧰 Additional context used
📓 Path-based instructions (1)
pyproject.toml

📄 CodeRabbit inference engine (CLAUDE.md)

pyproject.toml: Check pyproject.toml for supported Python versions before development
Always check pyproject.toml for existing dependencies before adding new ones
Always verify current library versions in pyproject.toml rather than assuming versions
Use pylint with source-roots = "src" configuration

Files:

  • pyproject.toml
🔇 Additional comments (1)
pyproject.toml (1)

50-53: LGTM! Correct fix for cross-platform greenlet availability.

The [asyncio] extra is the appropriate solution since:

  1. The project already uses async database drivers (aiosqlite, asyncpg) with SQLAlchemy.
  2. SQLAlchemy's base package only includes greenlet for specific platforms via markers, excluding s390x. The [asyncio] extra unconditionally pulls in greenlet as a dependency.
  3. This aligns with the existing async infrastructure already present in the project.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

SQLAlchemy's base dependencies specify greenlet with platform markers
that exclude s390x. Since we use async SQLAlchemy features (via
llama-stack), we need greenlet at runtime on all platforms.

The [asyncio] extra includes greenlet unconditionally, fixing s390x
builds that were failing with "No module named 'greenlet'".

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@shanemcd shanemcd force-pushed the shanemcd/sqlalchemy-asyncio branch from 4317ae8 to a1917cc Compare January 14, 2026 19:44
@tisnik
Copy link
Contributor

tisnik commented Jan 15, 2026

@shanemcd you would need to regenerate lockfile and requirements files as well:

uv sync --upgrade

and

make konflux-requirements

Btw is it really possible to run Llama-Stack on s390 platform? It depends on Pytorch and other "strange" libs... ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants