feat(python-sdk): Move to packaging with pyproject.toml#550
feat(python-sdk): Move to packaging with pyproject.toml#550evansims merged 3 commits intoopenfga:mainfrom
Conversation
|
Hey @abhiaagarwal thanks a lot for taking a stab at this! Would this change have any effect on downstream users installing the package we may need to consider? |
|
@evansims No breaking changes for downstream users. Modern pip versions (19.0+) fully support pyproject.toml, and the API/imports remain identical. The main benefit is eliminating transitive setuptools and build dependencies that currently get installed unnecessarily. Users will see cleaner dependency resolution and faster installs. Only consideration is users with very old pip (<18.1 from 2018) would need to upgrade, but this affects a negligible user base. |
|
I would be very surprised to see any breaking changes, this is only affects packaging. It will built/published as a wheel that any pip can install without any knowledge of pyproject.toml. |
abhiaagarwal
force-pushed
the
python-sdk-modern-packaging
branch
from
07231fd to
803cd91
Compare
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes migrate the Python client SDK's packaging and build system from legacy Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Make as Makefile
participant Docker as Docker (uv image)
participant uv as uv toolchain
Dev->>Make: make build-client-python / test-client-python
Make->>Docker: Run uv-based Alpine image
Docker->>uv: uv sync (install deps)
Docker->>uv: uv run ruff check/format, pytest, build, etc.
uv-->>Docker: Output results
Docker-->>Dev: Build/test artifacts and reports
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes were found. 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 (
|
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
Makefile(1 hunks)config/clients/python/.openapi-generator-ignore(1 hunks)config/clients/python/config.overrides.json(1 hunks)config/clients/python/template/pyproject.toml(0 hunks)config/clients/python/template/pyproject.toml.mustache(1 hunks)config/clients/python/template/requirements.mustache(0 hunks)config/clients/python/template/setup.mustache(0 hunks)config/clients/python/template/test-requirements.mustache(0 hunks)
💤 Files with no reviewable changes (4)
- config/clients/python/template/test-requirements.mustache
- config/clients/python/template/requirements.mustache
- config/clients/python/template/setup.mustache
- config/clients/python/template/pyproject.toml
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build-and-test-go-sdk
- GitHub Check: build-and-test-java-sdk
- GitHub Check: build-and-test-dotnet-sdk
🔇 Additional comments (10)
config/clients/python/.openapi-generator-ignore (1)
18-20: LGTM - Correctly ignoring legacy packaging files.The addition of
setup.py,requirements.txt, andtest-requirements.txtto the ignore patterns correctly prevents the OpenAPI generator from overwriting these legacy files, which aligns with the migration topyproject.toml-based packaging.config/clients/python/config.overrides.json (1)
17-19: LGTM - Properly configured templated pyproject.toml generation.The configuration correctly replaces the static
pyproject.tomlwith a mustache template that will generate the finalpyproject.tomlfile dynamically, enabling project-specific customization.config/clients/python/template/pyproject.toml.mustache (6)
22-55: LGTM - Well-structured project metadata section.The project configuration properly uses mustache templating for dynamic values and includes comprehensive metadata including classifiers, keywords, and license information.
56-64: LGTM - Comprehensive development dependencies.The development dependencies include all necessary tools for testing, linting, and type checking. The version constraints are appropriately set to ensure compatibility.
66-68: LGTM - Modern build system configuration.Using
hatchlingas the build backend is a good choice for modern Python packaging, providing better performance and fewer dependencies compared to setuptools.
71-136: LGTM - Well-configured development tools.The ruff configuration is comprehensive with appropriate exclusions, linting rules, and formatting settings. The line length of 88 and Python 3.10 target version align with modern Python practices.
137-146: LGTM - Comprehensive test configuration.The pytest configuration properly sets up test paths, coverage reporting, and asyncio settings, which are essential for testing the async Python SDK.
46-51: Verify dependency versions for security and compatibility.The runtime dependencies look reasonable, but please verify that these versions are current and secure:
aiohttp>=3.9.3python-dateutil>=2.9.0opentelemetry-api>=1.25.0urllib3>=1.26.19,<3#!/bin/bash # Check for latest versions and security advisories for core dependencies echo "Checking aiohttp..." curl -s https://pypi.org/pypi/aiohttp/json | jq '.info.version' echo "Checking python-dateutil..." curl -s https://pypi.org/pypi/python-dateutil/json | jq '.info.version' echo "Checking opentelemetry-api..." curl -s https://pypi.org/pypi/opentelemetry-api/json | jq '.info.version' echo "Checking urllib3..." curl -s https://pypi.org/pypi/urllib3/json | jq '.info.version' # Check for security advisories gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: PIP, package: "aiohttp") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }' gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: PIP, package: "urllib3") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Makefile (2)
125-129: LGTM - Modernized Python build process.The migration to
uvtoolchain with the Alpine-based image is an excellent improvement. TheUV_LINK_MODE=copyenvironment variable and sequential commands (uv sync,uv run ruff,uv build) properly implement the modern Python packaging workflow.
133-136: LGTM - Updated test workflow to use uv.The test target correctly uses the same
uv-based approach with proper dependency synchronization and tool execution. The pytest command maintains the existing coverage reporting requirements.
|
Sounds good, and makes total sense — thank you @abhiaagarwal and @Siddhant-K-code! Bandwidth has been a little tight this week, but give me a few to give this the attention it deserves and we'll follow up with you. Appreciate it! |
Signed-off-by: Abhi Agarwal <abhiaagarwal01@gmail.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
evansims
force-pushed
the
python-sdk-modern-packaging
branch
from
e02f61f to
304c41c
Compare
* chore: move to packaging with pyproject.toml Original PR: openfga/sdk-generator#550 * chore(ci): update build instructions --------- Co-authored-by: Abhi Agarwal <abhiaagarwal01@gmail.com>
3 participants
Description
Migrates to modern packaging for the OpenFGA python client. Mostly done to get rid of the transitive dependencies of
setuptoolsandbuildthat the current package installs.References
Closes #548
Review Checklist
mainSummary by CodeRabbit