Publish to pypi #466
Publish to pypi #466
Conversation
Reviewer's GuidePrepares the Python server package for distribution via PyPI by standardizing the package name/metadata, wiring telemetry to the new package identity, and adding a CI workflow to build and publish distributions to PyPI and TestPyPI. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
WalkthroughIntroduces a GitHub Actions workflow to automate Python package publishing to PyPI and TestPyPI. Adds package metadata, MIT license file, and updates package configuration in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The GitHub Actions workflow is configured with
on: push, which will build and upload artifacts on every branch push; consider narrowing this topushto main and/or tags to avoid unnecessary CI load. - The
Programming Language :: Python :: 3.14classifier inpyproject.tomlmay be premature; you might want to drop it until 3.14 is released and actually tested to avoid overstating compatibility.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The GitHub Actions workflow is configured with `on: push`, which will build and upload artifacts on every branch push; consider narrowing this to `push` to main and/or tags to avoid unnecessary CI load.
- The `Programming Language :: Python :: 3.14` classifier in `pyproject.toml` may be premature; you might want to drop it until 3.14 is released and actually tested to avoid overstating compatibility.
## Individual Comments
### Comment 1
<location> `.github/workflows/publish-pypi.yml:26-30` </location>
<code_context>
+ - name: Build a binary wheel and a source tarball
+ run: uv build
+
+ - name: Store the distribution packages
+ uses: actions/upload-artifact@v6
+ with:
+ name: python-package-distributions
+ path: Server/dist/
+
+ publish-to-pypi:
</code_context>
<issue_to_address>
**issue (bug_risk):** Artifact upload/download paths likely produce an extra nested directory, which will break `gh-action-pypi-publish` defaults.
`upload-artifact` preserves the `Server/dist/` directory, but the publish jobs download into `dist/` and rely on the default `packages-dir: dist/`. This will likely place wheels under `dist/Server/dist/...`, so `gh-action-pypi-publish` won’t see them.
To fix this, either:
- upload with `path: Server/dist/*`, or
- download into `Server/dist/` and set `packages-dir: Server/dist/` in the publish steps.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| - name: Store the distribution packages | ||
| uses: actions/upload-artifact@v6 | ||
| with: | ||
| name: python-package-distributions | ||
| path: Server/dist/ |
There was a problem hiding this comment.
issue (bug_risk): Artifact upload/download paths likely produce an extra nested directory, which will break gh-action-pypi-publish defaults.
upload-artifact preserves the Server/dist/ directory, but the publish jobs download into dist/ and rely on the default packages-dir: dist/. This will likely place wheels under dist/Server/dist/..., so gh-action-pypi-publish won’t see them.
To fix this, either:
- upload with
path: Server/dist/*, or - download into
Server/dist/and setpackages-dir: Server/dist/in the publish steps.
|
@msanatan exciting! Good thinking! |
| readme = "README.md" | ||
| license = "MIT" | ||
| authors = [ | ||
| {name = "Marcus Sanatan", email = "msanatan@gmail.com"}, |
There was a problem hiding this comment.
@dsarno @Scriptwonder is it OK to list you all as authors here?
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Server/src/core/telemetry.py (1)
63-63: Consider shortening the error message.The static analysis tool suggests avoiding long messages directly in the exception. You could shorten this to just the essential information or define a custom exception class.
🔎 Optional refactor to address static analysis hint
- raise FileNotFoundError("pyproject.toml not found for mcpforunityserver") + raise FileNotFoundError(f"pyproject.toml not found for {PACKAGE_NAME}")This approach uses the constant and keeps the message concise.
.github/workflows/publish-pypi.yml (1)
54-76: Consider limiting TestPyPI publishing frequency.The
publish-to-testpypijob runs on every push without any conditional filters. This could lead to:
- Excessive TestPyPI uploads (potentially hitting rate limits)
- Version conflicts if the version in
pyproject.tomldoesn't change between pushes- Cluttered TestPyPI with many development versions
Consider adding a condition to limit when this runs, such as:
- Only on pushes to specific branches (e.g.,
mainordevelop)- Only on pull request merges
- Or remove it entirely if manual testing on TestPyPI is sufficient
🔎 Example: Limit TestPyPI publishing to main branch
publish-to-testpypi: name: Publish Python 🐍 distribution 📦 to TestPyPI + if: github.ref == 'refs/heads/main' needs: - buildOr limit to non-tag pushes to avoid duplicate publishes:
publish-to-testpypi: name: Publish Python 🐍 distribution 📦 to TestPyPI + if: "!startsWith(github.ref, 'refs/tags/')" needs: - build
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/publish-pypi.yml(1 hunks)Server/LICENSE(1 hunks)Server/pyproject.toml(2 hunks)Server/src/core/telemetry.py(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
📚 Learning: 2025-10-13T13:41:00.086Z
Learnt from: JohanHoltby
Repo: CoplayDev/unity-mcp PR: 309
File: MCPForUnity/Editor/Helpers/ServerInstaller.cs:478-508
Timestamp: 2025-10-13T13:41:00.086Z
Learning: In the MCPForUnityTools feature (MCPForUnity/Editor/Helpers/ServerInstaller.cs), the design intentionally forces users to have only one .py file per MCPForUnityTools folder to keep file tracking simple. Package-style tools (subdirectories with __init__.py) are not supported.
Applied to files:
Server/src/core/telemetry.pyServer/pyproject.toml
📚 Learning: 2025-11-05T18:23:12.349Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 368
File: MCPForUnity/UnityMcpServer~/src/resources/menu_items.py:15-15
Timestamp: 2025-11-05T18:23:12.349Z
Learning: In Unity MCP, the `name` parameter in the `mcp_for_unity_resource` decorator is the external API name exposed to MCP clients (LLMs, AI agents). The command string passed to `async_send_command_with_retry` or `async_send_with_unity_instance` (e.g., "get_menu_items") is the internal command identifier that must match the C# side. These are decoupled, allowing external API naming to evolve independently of internal command routing.
Applied to files:
Server/pyproject.toml
📚 Learning: 2025-10-03T22:11:46.002Z
Learnt from: msanatan
Repo: CoplayDev/unity-mcp PR: 301
File: docs/CUSTOM_TOOLS.md:54-62
Timestamp: 2025-10-03T22:11:46.002Z
Learning: In Unity MCP, the `description` parameter in the `mcp_for_unity_tool` decorator is technically optional but should always be included as a best practice. Without it, there's a higher chance that MCP clients will not parse the tool correctly. All Unity MCP tools should include the description in the decorator for compatibility.
Applied to files:
Server/pyproject.toml
🪛 Ruff (0.14.8)
Server/src/core/telemetry.py
63-63: Avoid specifying long messages outside the exception class
(TRY003)
⏰ 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). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (8)
Server/LICENSE (1)
1-21: LGTM!The MIT License is properly formatted with the correct copyright year and holder.
Server/src/core/telemetry.py (1)
37-37: LGTM! Package name follows Python conventions.The package name change to all lowercase follows PEP 8 naming conventions for Python packages.
Server/pyproject.toml (4)
2-2: LGTM! Package name standardization.The package name change to lowercase follows Python packaging conventions and is consistent with the telemetry module updates.
6-11: LGTM! Proper package metadata.The license declaration and authors section are well-formatted and align with the new LICENSE file.
29-29: LGTM! Useful package metadata.The keywords and project URLs are relevant and will help users discover and contribute to the project.
Also applies to: 47-49
12-28: Remove or update the concern about Python 3.14.Python 3.14 was released on 7 October 2025, so including it in the classifiers is appropriate. The classifier on line 22 is valid and does not require changes.
.github/workflows/publish-pypi.yml (2)
32-53: LGTM! Proper PyPI publishing configuration.The conditional
if: startsWith(github.ref, 'refs/tags/')ensures that releases to PyPI only happen on tagged versions, which is the correct practice. The use of trusted publishing (OIDC) is also a security best practice.
6-31: LGTM! Well-structured build job.The build job correctly sets the working directory, uses modern tooling (uv), and uploads artifacts for downstream jobs. The cache dependency glob correctly references the existing
Server/uv.lockfile.
2 participants
We're now on pypi! It's another way to run the server: https://pypi.org/project/mcpforunityserver/
@dsarno @Scriptwonder I originally thought about adding this to get past the
uvxissues with Claude Desktop on Windows before David's fix. Hopefully users will appreciate an alternative all the same!Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.