-
Notifications
You must be signed in to change notification settings - Fork 61
infra: pypi trusted publishing ci #245
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
base: unstable
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| name: "Publish" | ||
|
|
||
| on: | ||
| push: | ||
| tags: | ||
| # Publish on any tag starting with a `v`, e.g., v0.1.0 | ||
| - v* | ||
|
|
||
| jobs: | ||
| run: | ||
| runs-on: ubuntu-latest | ||
| environment: | ||
| name: pypi | ||
| permissions: | ||
| id-token: write | ||
| contents: read | ||
| steps: | ||
| - name: Checkout | ||
| uses: actions/checkout@v6 | ||
| - name: Install uv | ||
| uses: astral-sh/setup-uv@v7 | ||
| - name: Install Python 3.12 | ||
| run: uv python install 3.12 | ||
| - name: Build | ||
| run: uv build | ||
| # Check that basic features work and we didn't miss to include crucial files | ||
| - name: Smoke test (wheel) | ||
| run: uv run --isolated --no-project --with dist/*.whl tests/smoke_test.py | ||
| - name: Smoke test (source distribution) | ||
| run: uv run --isolated --no-project --with dist/*.tar.gz tests/smoke_test.py | ||
| - name: Publish | ||
| run: uv publish | ||
|
Comment on lines
+31
to
+32
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider adding version tag verification before publishing. The workflow doesn't verify that the Git tag matches the version in 🔎 Proposed verification stepAdd this step before the publish step: - name: Verify version matches tag
run: |
TAG_VERSION="${GITHUB_REF#refs/tags/v}"
PACKAGE_VERSION=$(uv run python -c "import temoa; print(temoa.__version__)")
if [ "$TAG_VERSION" != "$PACKAGE_VERSION" ]; then
echo "Error: Tag version ($TAG_VERSION) doesn't match package version ($PACKAGE_VERSION)"
exit 1
fi
echo "✅ Version verified: $PACKAGE_VERSION"🤖 Prompt for AI Agents |
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,31 @@ | ||||||||||||||||||||||||||||||
| import subprocess | ||||||||||||||||||||||||||||||
| import sys | ||||||||||||||||||||||||||||||
| import temoa | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def test_import() -> None: | ||||||||||||||||||||||||||||||
| print(f"Importing temoa version: {temoa.__version__}") | ||||||||||||||||||||||||||||||
| assert temoa.__version__ is not None | ||||||||||||||||||||||||||||||
|
Comment on lines
+5
to
+7
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider strengthening the version assertion. The current assertion only checks for 🔎 Proposed enhancement def test_import() -> None:
print(f"Importing temoa version: {temoa.__version__}")
- assert temoa.__version__ is not None
+ assert temoa.__version__ is not None
+ assert isinstance(temoa.__version__, str)
+ assert len(temoa.__version__) > 0📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def test_cli() -> None: | ||||||||||||||||||||||||||||||
| print("Running temoa --version CLI command...") | ||||||||||||||||||||||||||||||
| result = subprocess.run(["temoa", "--version"], capture_output=True, text=True) | ||||||||||||||||||||||||||||||
| print(f"CLI output: {result.stdout.strip()}") | ||||||||||||||||||||||||||||||
| assert result.returncode == 0 | ||||||||||||||||||||||||||||||
| assert "Temoa Version:" in result.stdout | ||||||||||||||||||||||||||||||
| assert temoa.__version__ in result.stdout | ||||||||||||||||||||||||||||||
|
Comment on lines
+9
to
+15
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Add timeout to subprocess calls to prevent hangs. While the test logic is sound, subprocess calls without timeouts could cause the CI to hang indefinitely if the CLI becomes unresponsive. 🔎 Proposed fix def test_cli() -> None:
print("Running temoa --version CLI command...")
- result = subprocess.run(["temoa", "--version"], capture_output=True, text=True)
+ result = subprocess.run(["temoa", "--version"], capture_output=True, text=True, timeout=30)
print(f"CLI output: {result.stdout.strip()}")
assert result.returncode == 0
assert "Temoa Version:" in result.stdout
assert temoa.__version__ in result.stdout📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.10)11-11: Starting a process with a partial executable path (S607) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| def test_help() -> None: | ||||||||||||||||||||||||||||||
| print("Running temoa --help CLI command...") | ||||||||||||||||||||||||||||||
| result = subprocess.run(["temoa", "--help"], capture_output=True, text=True) | ||||||||||||||||||||||||||||||
| assert result.returncode == 0 | ||||||||||||||||||||||||||||||
| assert "The Temoa Project" in result.stdout | ||||||||||||||||||||||||||||||
|
Comment on lines
+17
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Add timeout to subprocess call. Same recommendation as 🔎 Proposed fix def test_help() -> None:
print("Running temoa --help CLI command...")
- result = subprocess.run(["temoa", "--help"], capture_output=True, text=True)
+ result = subprocess.run(["temoa", "--help"], capture_output=True, text=True, timeout=30)
assert result.returncode == 0
assert "The Temoa Project" in result.stdout📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.10)19-19: Starting a process with a partial executable path (S607) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if __name__ == "__main__": | ||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||
| test_import() | ||||||||||||||||||||||||||||||
| test_cli() | ||||||||||||||||||||||||||||||
| test_help() | ||||||||||||||||||||||||||||||
| print("\n✅ Smoke test passed!") | ||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||
| print(f"\n❌ Smoke test failed: {e}") | ||||||||||||||||||||||||||||||
| sys.exit(1) | ||||||||||||||||||||||||||||||
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.
🧹 Nitpick | 🔵 Trivial
Consider adding timeouts to smoke test steps.
The subprocess calls in
tests/smoke_test.pydon't have explicit timeouts, which could cause the workflow to hang if the CLI commands fail to respond. Consider adding atimeout-minutesdirective to these steps as a safeguard.🔎 Proposed enhancement
# Check that basic features work and we didn't miss to include crucial files - name: Smoke test (wheel) + timeout-minutes: 5 run: uv run --isolated --no-project --with dist/*.whl tests/smoke_test.py - name: Smoke test (source distribution) + timeout-minutes: 5 run: uv run --isolated --no-project --with dist/*.tar.gz tests/smoke_test.py🤖 Prompt for AI Agents