-
Notifications
You must be signed in to change notification settings - Fork 0
Add new pystub proxy support #5
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: main
Are you sure you want to change the base?
Conversation
WalkthroughRenames two public API functions ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
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.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/designer_plugin/d3sdk/client.py (1)
78-98: Update outdated API names in docstring.The docstring still references the old API names
d3_api_pluginandd3_api_apluginon line 88. These should be updated tod3_api_executeandd3_api_aexecute.🔎 Proposed fix
- 5. Sends it to Designer via d3_api_plugin or d3_api_aplugin + 5. Sends it to Designer via d3_api_execute or d3_api_aexecutesrc/designer_plugin/api.py (2)
222-269: Fix test mocks and comments to use renamed function.The function was renamed from
d3_api_plugintod3_api_execute, but tests intests/test_client.pystill reference the old name in 6 mock patches (lines 74, 117, 127, 137, 147, 165). These patches will fail because they're attempting to mock a non-existent function. Additionally, a comment insrc/designer_plugin/d3sdk/client.py:88references the old function name and needs updating.Update all test patches from
patch('designer_plugin.d3sdk.client.d3_api_plugin', ...)topatch('designer_plugin.d3sdk.client.d3_api_execute', ...)and update the source code comment.
128-174: Update docstring in src/designer_plugin/d3sdk/client.py to reflect renamed function.While the rename from
d3_api_aplugintod3_api_aexecutehas been completed successfully with all usages and imports updated, the docstring at line 88 inclient.pystill references the old function names: "Sends it to Designer via d3_api_plugin or d3_api_aplugin". This should be updated to reference the current function names (d3_api_executefor sync andd3_api_aexecutefor async).
🧹 Nitpick comments (1)
CHANGELOG.md (1)
8-13: Changelog entry is well-formatted and documents key changes.The 1.3.0 entry follows the Keep a Changelog format correctly and captures the API renames and pystub documentation update. However, consider clarifying that these are breaking changes (old API names are removed, not available alongside new names) to help users understand migration urgency.
Consider rephrasing the API rename entries to explicitly indicate removal for clarity:
🔎 Suggested enhancement for breaking change clarity
### Changed -- `d3_api_plugin` has been renamed to `d3_api_execute`. -- `d3_api_aplugin` has been renamed to `d3_api_aexecute`. +- **BREAKING**: `d3_api_plugin` has been removed; use `d3_api_execute` instead. +- **BREAKING**: `d3_api_aplugin` has been removed; use `d3_api_aexecute` instead. - Updated documentation to reflect `pystub` proxy support.This makes it explicit that these are not backward-compatible changes and require code updates from users of the old API names.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
CHANGELOG.mdREADME.mdpyproject.tomlsrc/designer_plugin/api.pysrc/designer_plugin/d3sdk/client.pysrc/designer_plugin/d3sdk/session.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.243Z
Learning: The `designer-plugin-pystub` package is a separate PyPI package (not part of the designer-plugin repository) that provides type stubs for Designer's Python API. It is installed separately via `pip install designer-plugin-pystub` and imported under `TYPE_CHECKING` blocks for IDE autocomplete.
📚 Learning: 2025-12-01T16:07:14.243Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.243Z
Learning: The `designer-plugin-pystub` package is a separate PyPI package (not part of the designer-plugin repository) that provides type stubs for Designer's Python API. It is installed separately via `pip install designer-plugin-pystub` and imported under `TYPE_CHECKING` blocks for IDE autocomplete.
Applied to files:
README.mdpyproject.toml
🧬 Code graph analysis (1)
src/designer_plugin/d3sdk/session.py (2)
src/designer_plugin/api.py (3)
d3_api_aexecute(128-173)d3_api_arequest(97-123)d3_api_execute(222-269)src/designer_plugin/d3sdk/function.py (2)
payload(207-223)payload(350-368)
🔇 Additional comments (8)
pyproject.toml (1)
7-7: Version bump aligns with feature changes and breaking API removals.The minor version bump to 1.3.0 is appropriate for feature additions (pystub proxy support) and breaking API changes (d3_api_plugin → d3_api_execute, d3_api_aplugin → d3_api_aexecute). Since these are breaking changes, confirm that users are notified in release notes and the changelog clearly indicates this is a breaking release.
README.md (1)
103-103: Client and Functional API examples correctly updated with new import.Both examples consistently use the new simplified import pattern
from designer_plugin.pystub import *, maintaining consistency with the documented pattern in the "Stub file" section.Also applies to: 187-187
src/designer_plugin/d3sdk/session.py (3)
10-18: LGTM: Imports updated correctly.The imports correctly reflect the renamed API functions (
d3_api_aexecuteandd3_api_execute).
120-120: LGTM: Synchronous execute call updated.The synchronous session correctly calls the renamed
d3_api_executefunction.
273-273: LGTM: Asynchronous execute call updated.The asynchronous session correctly calls the renamed
d3_api_aexecutefunction.src/designer_plugin/d3sdk/client.py (3)
15-20: LGTM: Imports updated correctly.The imports correctly reflect the renamed API functions (
d3_api_aexecuteandd3_api_execute).
115-118: LGTM: Asynchronous wrapper updated correctly.The async wrapper correctly calls the renamed
d3_api_aexecutefunction.
133-136: LGTM: Synchronous wrapper updated correctly.The sync wrapper correctly calls the renamed
d3_api_executefunction.
edf2221 to
a4b05ba
Compare
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/designer_plugin/d3sdk/client.py (1)
78-98: Update the docstring to reference the new API names.The docstring on line 88 still references the old API names. Update it to reflect the renamed functions.
🔎 Suggested fix
3. Builds a script string in the form: "return plugin.{method_name}({args})" 4. Creates a PluginPayload with the script and module information - 5. Sends it to Designer via d3_api_plugin or d3_api_aplugin + 5. Sends it to Designer via d3_api_execute or d3_api_aexecute 6. Returns the result from the remote execution
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
CHANGELOG.mdREADME.mdpyproject.tomlsrc/designer_plugin/api.pysrc/designer_plugin/d3sdk/client.pysrc/designer_plugin/d3sdk/session.pytests/test_client.py
🚧 Files skipped from review as they are similar to previous changes (4)
- README.md
- CHANGELOG.md
- pyproject.toml
- src/designer_plugin/d3sdk/session.py
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.243Z
Learning: The `designer-plugin-pystub` package is a separate PyPI package (not part of the designer-plugin repository) that provides type stubs for Designer's Python API. It is installed separately via `pip install designer-plugin-pystub` and imported under `TYPE_CHECKING` blocks for IDE autocomplete.
🧬 Code graph analysis (1)
src/designer_plugin/d3sdk/client.py (2)
src/designer_plugin/api.py (3)
d3_api_aexecute(128-173)d3_api_aregister_module(176-217)d3_api_execute(222-269)src/designer_plugin/models.py (1)
PluginResponse(45-89)
🔇 Additional comments (6)
tests/test_client.py (1)
74-74: LGTM! Test patches updated consistently.All patch targets correctly updated from
d3_api_plugintod3_api_executeto align with the API rename. The test logic remains unchanged, ensuring continued validation of the updated API surface.Also applies to: 117-117, 127-127, 137-137, 147-147, 165-165
src/designer_plugin/api.py (2)
128-173: LGTM! Clean async API rename.Function renamed from
d3_api_aplugintod3_api_aexecutewith signature and behavior preserved.
222-269: LGTM! Clean sync API rename.Function renamed from
d3_api_plugintod3_api_executewith signature and behavior preserved.src/designer_plugin/d3sdk/client.py (3)
16-20: LGTM! Imports updated correctly.Import statements updated to reference the renamed API functions
d3_api_aexecuteandd3_api_execute.
115-118: LGTM! Async call site updated correctly.Call updated to use the renamed
d3_api_aexecutefunction.
133-136: LGTM! Sync call site updated correctly.Call updated to use the renamed
d3_api_executefunction.
chrisnashdisguise
left a comment
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.
So the code which implements the proxy will be in the pystub repository? Should there be some dependency version change for that repo here somewhere?
Co-authored-by: Chris Nash <chris.nash@disguise.one>
The version change will be on pystub repository itself. I'm currently testing with test-pypi server to make sure everything is working as expected. |
Should this repo have a dependency on the pystub repo? Even if it doesn't directly use it, we might need to enforce version compatibility in cases like this? |
Add pystub proxy support
Summary
This PR introduces a Python proxy object for the stub file
d3.pyi, removing the need for clients to hide thed3module behindTYPE_CHECKING.Previously, this limitation made it hard to reference the d3 plugin type as a function return type.
The new proxy resolves that issue and improves the overall typing experience.
Old
New
In addition, the raw API interface names have been updated to align with the existing naming conventions. While it's to deprecate the old APIs, the previous release went out shortly before Christmas, so I just removed instead, keeping the surface area clean and consistent.
Changes
d3_api_plugintod3_api_execute.d3_api_aplugintod3_api_aexecute.Summary by CodeRabbit
Changed
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.