-
Notifications
You must be signed in to change notification settings - Fork 0
Add d3sdk #4
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
Add d3sdk #4
Conversation
WalkthroughAdds a Designer plugin SDK: Pydantic models, sync/async HTTP API, session managers, metaclass-based remote client and function decorators, AST utilities to convert Py3→Py2.7 code, package logger, packaging/metadata updates, expanded docs, and extensive tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User code
participant SDK as designer_plugin.d3sdk
participant API as designer_plugin.api
participant Designer as Designer Server
participant Models as designer_plugin.models
User->>SDK: define class / decorate function
User->>SDK: open session (D3Session / D3AsyncSession)
SDK->>API: register modules (d3_api_register_module / d3_api_aregister_module)
API->>Designer: POST /api/plugin/register (RegisterPayload)
Designer-->>API: PluginRegisterResponse
API->>Models: validate -> PluginRegisterResponse
API-->>SDK: registration result
User->>SDK: call wrapped method or script
SDK->>SDK: build PluginPayload (moduleName or script)
SDK->>API: execute plugin (d3_api_plugin / d3_api_aplugin)
API->>Designer: POST /api/plugin/execute (PluginPayload)
Designer-->>API: PluginResponse (status, logs, returnValue)
API->>Models: validate/parse -> PluginResponse / PluginError
API-->>SDK: typed result or raise PluginException
SDK-->>User: returnValue or propagate PluginException
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ 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 |
9b8d5e1 to
a310700
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: 10
🧹 Nitpick comments (15)
pyproject.toml (1)
14-18: Consider movingtypes-requeststo dev dependencies.
types-requestsis a type stub package used for static type checking, not at runtime. It should typically be in thedevdependency group rather than runtimedependencies.dependencies = [ "aiohttp>=3.13.2", "pydantic>=2.12.4", "requests>=2.32.5", - "types-requests>=2.32.4.20250913", "zeroconf>=0.39.0", ]Add to dev dependencies:
[dependency-groups] dev = [ "mypy>=1.18.2", "pre-commit>=4.4.0", "pytest>=9.0.1", "ruff>=0.14.5", "twine>=6.2.0", + "types-requests>=2.32.4.20250913", ]src/designer_plugin/logger.py (1)
64-68: Consider usingTextIOfor the stream parameter type.The
streamparameter accepts a stream object but is typed asAny. UsingTextIOfromtypingwould provide better type safety and IDE assistance.-from typing import Any +from typing import TextIO def enable_debug_logging( level: int = logging.DEBUG, - stream: Any | None = None, + stream: TextIO | None = None, format_string: str | None = None, ) -> None:README.md (1)
94-94: Consider using a proper heading for "Example".Static analysis flagged bold text being used instead of a heading. For better document structure and accessibility, consider using
### Exampleinstead of**Example**.-**Example** +### ExampleThis also applies to line 180.
src/designer_plugin/api.py (4)
89-94: Add error handling for HTTP failures.The function doesn't handle HTTP errors or non-JSON responses, which could lead to cryptic errors. Consider checking the response status and handling potential JSON decode errors.
response = requests.request( method, url, **kwargs, ) - return response.json() + response.raise_for_status() + return response.json()
117-123: Consider documenting session creation overhead.Creating a new
ClientSessionper request has overhead. While acceptable for low-frequency calls, callers making many requests should be aware. The higher-level session classes ind3sdk/session.pyappear to manage this appropriately.Consider adding a note to the docstring:
"""Make an asynchronous HTTP request to Designer API. + Note: + Creates a new ClientSession per call. For repeated requests, + consider using D3AsyncSession which manages connection pooling. + Args:
160-161: Avoid usingprint()in library code.Using
print()directly bypasses the logging system and can interfere with applications that capture stdout. Consider using the logger or letting callers handle output themselves.if plugin_response.pythonLog: - print(plugin_response.pythonLog) + logger.info("Python log output:\n%s", plugin_response.pythonLog)This also applies to the sync version on lines 254-255.
101-103: Private type_RequestOptionsmay be unstable.Using
aiohttp.client._RequestOptionsreferences a private type that could change without notice. Consider usingAnyfor**kwargsto avoid coupling to internal aiohttp types.async def d3_api_arequest( method: Method, hostname: str, port: int, url_endpoint: str, - **kwargs: Unpack[aiohttp.client._RequestOptions], + **kwargs: Any, ) -> Any:src/designer_plugin/models.py (1)
84-88: Debug string format is unusual but functional.The f-string fill/align syntax
{"json ":{'='}<60}produces output likejson ====.... This works but may be confusing to readers unfamiliar with advanced f-string formatting.Consider a more readable approach:
def debug_string(self) -> str: - return f""" -{"json ":{'='}<60} -{self.model_dump_json(indent=2)}""" + return f""" +{'=' * 60} json +{self.model_dump_json(indent=2)}"""tests/test_ast_utils.py (1)
711-747:test_get_source_dedents_codetest logic could be more robust.The test relies on finding a specific docstring pattern in the source file, which is fragile. If the file format changes, the test might fail or pass incorrectly.
Consider a more deterministic check:
def test_get_source_dedents_code(self): """Test that extracted source code is dedented.""" def nested_function(): frame = inspect.currentframe() return get_source(frame) if frame else None source = nested_function() # Verify the source is dedented (doesn't start with leading whitespace) assert source is not None - # The source should contain the entire file, dedented - lines = source.split('\n') - # Find lines that should be at the start of the file - for line in lines: - if line.startswith('"""Tests for AST'): - # This line should be at column 0 after dedenting - assert line[0] != ' ' - break + # First non-empty line should not start with whitespace + first_line = next((line for line in source.split('\n') if line.strip()), '') + assert not first_line.startswith(' ') and not first_line.startswith('\t')src/designer_plugin/d3sdk/ast_utils.py (2)
15-28:get_sourcereturns entire file source, not frame-specific source.The function uses
inspect.findsource(frame)which returns all source lines of the file containing the frame, not just the source at the frame's location. The docstring says "Extract and dedent source code from a frame object" which may be misleading—it extracts the entire file's source. This appears intentional based on usage inclient.py, but the docstring could be clarified.Consider updating the docstring to clarify this returns the entire file's source:
def get_source(frame: types.FrameType) -> str | None: - """Extract and dedent source code from a frame object. + """Extract and dedent the entire source file containing a frame object. Args: - frame: The frame object to extract source code from + frame: The frame object whose containing file's source will be extracted Returns: - Dedented source code as a string, or None if source cannot be found + Dedented source code of the entire file as a string, or None if source cannot be found
360-386: Substring matching for package filtering may be overly broad.The filtering logic uses
infor substring matching (e.g.,d3blobgen_package_name in module), which could inadvertently filter out packages likemy_d3blobgen_utilsortyping_extensions. Whiletyping_extensionsshould probably be filtered, user packages containing these substrings might be unintentionally excluded.Consider using more precise matching:
- if any(d3blobgen_package_name in module for module in imported_modules): + if any(module == d3blobgen_package_name or module.startswith(f"{d3blobgen_package_name}.") for module in imported_modules): continue - if any(typing_package_name in module for module in imported_modules): + if any(module == typing_package_name or module.startswith(f"{typing_package_name}.") for module in imported_modules): continueAlternatively, if
typing_extensionsshould also be filtered, explicitly add it to the exclusion list.src/designer_plugin/d3sdk/client.py (1)
359-372: Session context managers modify shared state without cleanup.When
module_nameis provided, it's set onself.module_name(creating an instance attribute), but this is never reset in thefinallyblock. If the same instance is used for multiple sessions with different module names, the override persists.Store and restore the original module_name:
async def async_session( self, hostname: str, port: int = D3_PLUGIN_DEFAULT_PORT, register_module: bool = True, module_name: str | None = None, ) -> Any: + original_module_name = getattr(self, 'module_name', self.__class__.module_name) try: if module_name: self.module_name = module_name self._hostname = hostname self._port = port if register_module: await self._aregister(hostname, port) logger.debug("Entering D3PluginModule context") yield self finally: + self.module_name = original_module_name self._hostname = None self._port = None logger.debug("Exiting D3PluginModule context")src/designer_plugin/d3sdk/function.py (1)
96-99: Body extraction can be simplified.The loop concatenating strings with
+=creates intermediate strings. A more Pythonic approach would use a generator or list comprehension.# Convert back to source code - body: str = "" - for stmt in body_nodes: - body += ast.unparse(stmt) + "\n" + body: str = "\n".join(ast.unparse(stmt) for stmt in body_nodes)Same applies to
body_py27at lines 109-111.src/designer_plugin/d3sdk/session.py (2)
300-319: Asyncregister_all_modulesperforms sequential registration.The async version registers modules sequentially using
awaitin a loop. For many modules, concurrent registration withasyncio.gatherwould be more efficient.Consider parallel registration:
async def register_all_modules( self, timeout_sec: float | None = None ) -> dict[str, bool]: + import asyncio + modules: list[str] = list(D3Function._available_d3functions.keys()) - register_success: dict[str, bool] = {} - for module_name in modules: - is_registered: bool = await self.register_module(module_name, timeout_sec) - register_success[module_name] = is_registered - return register_success + results = await asyncio.gather( + *[self.register_module(m, timeout_sec) for m in modules], + return_exceptions=True + ) + return { + m: r if isinstance(r, bool) else False + for m, r in zip(modules, results) + }Note: This changes error handling behavior—exceptions would be captured rather than propagated immediately.
84-86: Empty__exit__method - consider documenting intent.The
__exit__method does nothing. While this is valid, adding a brief comment or docstring explaining why no cleanup is needed would improve maintainability.def __exit__(self, _type: Any, _value: Any, _traceback: Any) -> None: - """Exit context manager.""" - pass + """Exit context manager. + + No cleanup required - module registrations persist on Designer side. + """
📜 Review details
Configuration used: CodeRabbit 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 (16)
.gitignore(1 hunks)README.md(3 hunks)pyproject.toml(4 hunks)src/designer_plugin/__init__.py(1 hunks)src/designer_plugin/api.py(1 hunks)src/designer_plugin/d3sdk/__init__.py(1 hunks)src/designer_plugin/d3sdk/ast_utils.py(1 hunks)src/designer_plugin/d3sdk/client.py(1 hunks)src/designer_plugin/d3sdk/function.py(1 hunks)src/designer_plugin/d3sdk/session.py(1 hunks)src/designer_plugin/designer_plugin.py(2 hunks)src/designer_plugin/logger.py(1 hunks)src/designer_plugin/models.py(1 hunks)tests/test_ast_utils.py(1 hunks)tests/test_core.py(1 hunks)tests/test_plugin.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (8)
tests/test_core.py (1)
src/designer_plugin/d3sdk/function.py (10)
D3Function(213-343)D3PythonScript(127-210)FunctionInfo(28-48)d3function(382-422)d3pythonscript(348-378)extract_function_info(51-120)get_all_d3functions(466-477)get_all_modules(480-486)name(159-165)module_name(302-308)
src/designer_plugin/d3sdk/__init__.py (3)
src/designer_plugin/d3sdk/client.py (2)
D3PluginClient(284-446)session(375-407)src/designer_plugin/d3sdk/function.py (7)
add_packages_in_current_file(425-451)d3function(382-422)d3pythonscript(348-378)get_all_d3functions(466-477)get_all_modules(480-486)get_register_payload(263-269)get_register_payload(454-463)src/designer_plugin/d3sdk/session.py (2)
D3AsyncSession(178-319)D3Session(45-175)
tests/test_ast_utils.py (1)
src/designer_plugin/d3sdk/ast_utils.py (9)
ConvertToPython27(89-250)convert_class_to_py27(279-293)convert_function_to_py27(255-276)filter_base_classes(49-58)filter_init_args(61-84)find_packages_in_current_file(298-387)get_class_node(31-44)get_source(15-28)visit_AsyncFunctionDef(115-143)
src/designer_plugin/d3sdk/function.py (2)
src/designer_plugin/d3sdk/ast_utils.py (2)
convert_function_to_py27(255-276)find_packages_in_current_file(298-387)src/designer_plugin/models.py (2)
PluginPayload(152-170)RegisterPayload(173-184)
src/designer_plugin/api.py (4)
src/designer_plugin/models.py (9)
PluginError(90-95)PluginException(107-147)PluginPayload(152-170)PluginRegisterResponse(98-103)PluginResponse(45-87)RegisterPayload(173-184)debug_string(84-87)debug_string(165-170)debug_string(179-184)src/designer_plugin/d3sdk/session.py (2)
request(122-133)request(221-239)src/designer_plugin/d3sdk/client.py (1)
session(375-407)src/designer_plugin/d3sdk/function.py (2)
payload(197-210)payload(328-343)
src/designer_plugin/d3sdk/session.py (3)
src/designer_plugin/api.py (6)
Method(59-66)d3_api_aplugin(128-171)d3_api_aregister_module(174-215)d3_api_arequest(97-123)d3_api_plugin(220-265)d3_api_request(69-94)src/designer_plugin/d3sdk/function.py (5)
D3Function(213-343)module_name(302-308)payload(197-210)payload(328-343)get_module_register_payload(272-299)src/designer_plugin/models.py (3)
PluginPayload(152-170)PluginResponse(45-87)RegisterPayload(173-184)
src/designer_plugin/d3sdk/client.py (3)
src/designer_plugin/api.py (4)
d3_api_aplugin(128-171)d3_api_aregister_module(174-215)d3_api_plugin(220-265)d3_api_register_module(268-311)src/designer_plugin/d3sdk/ast_utils.py (5)
convert_class_to_py27(279-293)filter_base_classes(49-58)filter_init_args(61-84)get_class_node(31-44)get_source(15-28)src/designer_plugin/models.py (3)
PluginPayload(152-170)PluginResponse(45-87)RegisterPayload(173-184)
src/designer_plugin/__init__.py (1)
src/designer_plugin/models.py (7)
PluginException(107-147)PluginPayload(152-170)PluginRegisterResponse(98-103)PluginResponse(45-87)PluginStatus(30-37)PluginStatusDetail(23-27)RegisterPayload(173-184)
🪛 markdownlint-cli2 (0.18.1)
README.md
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
180-180: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (27)
.gitignore (1)
175-177: LGTM!Standard addition for ignoring macOS-specific metadata files.
pyproject.toml (1)
28-30: LGTM!Correct PEP 561 configuration for typed package support.
src/designer_plugin/__init__.py (1)
1-28: LGTM!Clean public API surface with proper
__all__declaration. All imported symbols are correctly re-exported for package consumers.src/designer_plugin/designer_plugin.py (1)
45-47: LGTM!Formatting improvement for better readability.
src/designer_plugin/logger.py (1)
120-137: LGTM!Clean implementation of logging toggle functions following Python logging best practices.
README.md (1)
234-250: LGTM!Logging documentation is clear and provides both quick-start and production-appropriate configuration options.
src/designer_plugin/api.py (1)
29-54: LGTM!Clean URL helper functions with proper documentation.
src/designer_plugin/models.py (4)
1-18: LGTM! Well-structured module header and constants.The endpoint constants are clearly defined and provide good defaults for the plugin API.
23-38: LGTM! Clean Pydantic model definitions for status types.The
PluginStatusDetailandPluginStatusmodels are well-documented with appropriate field descriptions.
106-148: Well-designed exception dataclass with informative string representation.The
PluginExceptioncorrectly captures traceback and provides a detailed__str__implementation. Good use of__post_init__for traceback capture.
152-171: LGTM! Payload models are well-structured.The
PluginPayloadandRegisterPayloadclasses provide clean abstractions for script execution and module registration.tests/test_plugin.py (1)
1-5: LGTM! Consistent license header added.The MIT license header matches the format used in other files in this PR.
src/designer_plugin/d3sdk/__init__.py (1)
1-27: LGTM! Clean package initialization with well-defined public API.The
__all__list correctly exposes the primary SDK components: session classes, client base, decorators, and utility functions. The imports are logically grouped by their source modules.tests/test_core.py (5)
1-16: LGTM! Well-organized imports and license header.The imports cover all necessary components from the D3 SDK function module.
67-133: Thorough test coverage forextract_function_info.The tests cover simple functions, functions with args, decorated functions, typed functions, and complex type annotations. The Python 2.7 conversion validation is particularly valuable.
269-292: Test for D3PythonScript payload generation looks correct.The tests verify that the payload contains variable assignments and function body, and that
moduleNameis None for standalone scripts.
43-65: I'll help you verify the review comment about test isolation in the D3Function registry. Let me start by exploring the codebase to understand the registry mechanism and how it's managed across tests.
<function_calls>
#!/bin/bashFirst, let's understand the repository structure
find . -type f -name "*.py" | grep -E "(test_|conftest|__)" | head -20
</function_calls>Now let me search for the D3Function registry and how it's managed:
<function_calls>
#!/bin/bashSearch for available_d3functions and registry-related patterns
rg -n "available_d3functions" --type=py -C3
</function_calls>Let me also search for the d3function decorator implementation and registry clearing logic:
<function_calls>
#!/bin/bashSearch for d3function decorator and registry patterns
rg -n "d3function|registry|clear" --type=py -C2 | head -100
</function_calls>Let me look at the test file itself and check for any setup/teardown:
<function_calls>
#!/bin/bashFind and examine the test file
cat -n tests/test_core.py | head -80
</function_calls>Let me also search for the D3Function class definition:
<function_calls>
#!/bin/bashSearch for D3Function class and global state management
rg -n "class D3Function|def d3function|D3_FUNCTIONS|_registry" --type=py -B2 -A5
</function_calls>
240-255: Equality test shows intentional design choice, not a bug.The D3Function equality implementation is based on function name only (not module), making two instances that wrap the same function but register to different modules equal and have the same hash. This behavior is intentional—validated by the test comment "Should be equal and have same hash because they wrap the same function"—and does not cause practical issues because functions are isolated in module-keyed sets (
defaultdict[str, set[D3Function]]). Each module maintains its own registry, so cross-module collisions don't occur. While the design choice could theoretically lead to unexpected behavior if code patterns change or sets from different modules are combined, the current architecture prevents this scenario. The test confirms this is working as designed.tests/test_ast_utils.py (7)
1-23: LGTM! Well-organized imports and license header.The imports cover all AST utility functions being tested.
25-192: Excellent test coverage forConvertToPython27transformer.The tests thoroughly cover:
- Return type annotation removal
- Argument annotation removal
- Async to sync conversion
- Await expression removal
- Annotated assignment conversion
- Complex type annotations
- Nested async functions
This is comprehensive coverage for the Python 3 → 2.7 transformation.
193-528: Thorough f-string conversion tests.The f-string test suite covers:
- Basic f-strings
- Format specifications (
.2f)- Conversion flags (
!r,!s,!a)- Multiple expressions
- Literal braces (escaped)
- Complex expressions (
len(items))- Combined conversion flag and format spec
- Nested f-strings in annotated assignments
This is excellent coverage for f-string →
.format()conversion.
586-615: Good test for class method conversion.The test verifies that both async and regular methods in a class are properly converted to Python 2.7 compatible format.
893-949: Comprehensive tests forfind_packages_in_current_file.The tests properly verify:
- Import discovery from the current file
- Exclusion of typing imports
- Exclusion of d3blobgen imports
- Self-exclusion of the function
- Unique and sorted results
This covers the key filtering logic for Python 2.7 compatible import discovery.
1130-1131: LGTM! Proper test runner configuration.The
pytest.maincall with verbose flag allows running tests directly.
1082-1105: The review comment is based on an incorrect assumption. Theconvert_class_to_py27function DOES process nested classes indirectly through theConvertToPython27transformer's recursive traversal.Verification shows that when
convert_class_to_py27is called on the outer class, it processes the outer method viaconvert_function_to_py27, which usesast.NodeTransformer.generic_visit(). This recursively processes all child nodes, including nestedClassDefnodes and their methods. The test confirms that inner class method annotations are properly removed.However, the test could be improved by adding explicit assertions to verify inner class conversion (e.g.,
assert inner_method.returns is None), even though the conversion is actually happening.Likely an incorrect or invalid review comment.
src/designer_plugin/d3sdk/ast_utils.py (1)
194-251: Well-implemented f-string to.format()conversion.The
visit_JoinedStrmethod handles f-string conversion comprehensively, including:
- Literal string escaping of braces
- Conversion specifiers (
!r,!s,!a)- Simple format specs (e.g.,
:.2f)- Recursive transformation of expression values
The
NotImplementedErrorfor complex format specs is a reasonable fallback.src/designer_plugin/d3sdk/session.py (1)
29-43: Well-structured base class for session management.The
D3SessionBaseclass cleanly encapsulates shared state (hostname, port, context_modules) for both sync and async session implementations, following DRY principles.
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
♻️ Duplicate comments (1)
README.md (1)
220-220: Correct the PluginResponse import to use the public API.This import is incorrect;
PluginResponseis not exported fromdesigner_plugin.d3sdk. Per verification in the past review, it should be imported from the top-leveldesigner_pluginpackage.Apply this fix:
- from designer_plugin.d3sdk import PluginResponse + from designer_plugin import PluginResponse
🧹 Nitpick comments (1)
README.md (1)
94-94: Convert bold emphasis to markdown headings for proper structure.Lines 94 and 180 use
**Example**(bold emphasis) where markdown headings should be used per linter rules (MD036). Convert these to### Examplefor consistency and proper document hierarchy.Apply this diff:
-**Example** +### ExampleThis applies to both line 94 and line 180.
Also applies to: 180-180
📜 Review details
Configuration used: CodeRabbit 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 (1)
README.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
180-180: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (1)
README.md (1)
1-90: Comprehensive and well-structured documentation.The README provides excellent coverage of the new d3sdk features with clear examples for both sync and async patterns. The TYPE_CHECKING pattern for IDE support is well-documented, and the distinction between Client SDK and Function SDK use cases is clearly explained.
Also applies to: 151-230, 234-256
82f790b to
a4d62ec
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: 1
🧹 Nitpick comments (1)
README.md (1)
94-94: Convert bold text to markdown headings.Lines 94 and 180 use bold
**Example**instead of markdown heading syntax. Convert these to proper heading markup (e.g.,### Example) for consistency and to comply with markdown linting rules (MD036).- **Example** + ### ExampleApply this change to both occurrences (lines 94 and 180).
Also applies to: 180-180
📜 Review details
Configuration used: CodeRabbit 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 (1)
README.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
180-180: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (1)
README.md (1)
65-250: Documentation of Execute Python section is comprehensive and well-structured.The newly added Execute Python section (and its subsections) provides excellent coverage of:
- Stub file installation for IDE support (TYPE_CHECKING pattern)
- Client SDK with clear sync/async examples
- Function SDK with decorator patterns and session API details
- Logging configuration
The documentation clearly explains Python 2.7 compatibility considerations, session management, and when to use
session.rpc()vssession.plugin(). Code examples demonstrate both synchronous and asynchronous patterns effectively. The PluginResponse structure is well-documented.
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
🧹 Nitpick comments (1)
README.md (1)
94-94: Use proper markdown headings instead of emphasis for section titles.Lines 94 and 180 use
**Example**(bold emphasis) as section headers, which violates markdown style guidelines (MD036). Use proper heading syntax instead.Apply this diff to use proper markdown headings:
- **Example** + ### ExampleThis should be applied to both line 94 (in the Client API section) and line 180 (in the Functional API section).
Also applies to: 180-180
📜 Review details
Configuration used: CodeRabbit 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 (1)
README.md(2 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
180-180: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (2)
README.md (2)
220-220: PluginResponse import path corrected.The import has been fixed from the incorrect
designer_plugin.d3sdkto the correct public APIdesigner_plugin.PluginResponse.
85-85: Verify thatdesigner_plugin.pystub.d3import path is valid for the external package.Lines 85, 100, and 186 reference
from designer_plugin.pystub.d3 import *withinTYPE_CHECKINGblocks. While the README documents installingdesigner-plugin-pystubfrom PyPI (line 78), the import path validity depends on how that package is structured.Type checkers (mypy, pyright, etc.) must be able to resolve this import path after the stub package is installed. If the PyPI package does not export this path, these imports will fail during type checking.
Please verify the correct import path by:
- Installing
designer-plugin-pystubfrom PyPI- Confirming the import path works with a type checker (e.g.,
mypy --strict)- Checking if the package uses PEP 561 stubs format and exports the expected module structure
Alternatively, if you control the
designer-plugin-pystubpackage, ensure it exportsdesigner_plugin.pystub.d3as documented in the README examples.Also applies to: 100-100, 186-186
dbe41bf to
95dd216
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: 6
♻️ Duplicate comments (2)
src/designer_plugin/d3sdk/client.py (2)
41-64: Document supported argument types forrepr()serialization.The
build_payloadfunction usesrepr()to serialize arguments, which works for basic Python types but fails silently for custom objects (producing non-executable code like<ClassName object at 0x...>).Add documentation specifying supported types or implement validation:
def build_payload( self: Any, method_name: str, args: tuple[Any, ...], kwargs: dict[str, Any] ) -> PluginPayload[Any]: """Build plugin payload for remote method execution. + + Note: Arguments are serialized using repr(). Supported types include: + int, float, bool, None, str, list, dict, tuple, and objects whose + __repr__ returns valid Python literal code. Args:
233-239: Comment updated but behavior unchanged - single-underscore methods still wrapped.The comment now says "Skip internal framework methods" but still only checks
not attr_name.startswith("__"). Single-underscore methods (e.g.,_helper()) will still be wrapped for remote execution, which may expose internal implementation details.If this behavior is intentional, the comment is accurate. If single-underscore methods should remain local, update the condition:
- if callable(attr_value) and not attr_name.startswith("__"): + if callable(attr_value) and not attr_name.startswith("_"):
🧹 Nitpick comments (15)
README.md (2)
94-94: Use markdown heading syntax for "Example" section labels.Line 94 uses bold emphasis (
**Example**) instead of a markdown heading. For consistency and proper document structure, use### Exampleinstead.-**Example** +### Example
180-180: Use markdown heading syntax for "Example" section labels.Line 180 uses bold emphasis (
**Example**) instead of a markdown heading. For consistency and proper document structure, use### Exampleinstead.-**Example** +### Examplesrc/designer_plugin/d3sdk/ast_utils.py (3)
189-245: Consider handling nested f-strings in format specs.Lines 222-224 silently pass when encountering complex format specs (e.g., nested expressions in f-string format specs like
f"{x:{width}}"). This could produce incorrect output without warning.else: - # For more complex specs we could fall back, but let's keep it simple - pass + # Complex format specs with nested expressions are not supported + raise NotImplementedError( + f"Complex f-string format spec not supported: {ast.dump(fspec)}" + )
286-288: Missing handling for nested class methods.The function only iterates over
class_node.bodydirectly, but classes can have nested classes or other constructs containing functions. Consider usingast.walkfor comprehensive conversion, or document the limitation.
345-347: TYPE_CHECKING detection may miss aliased imports.The
is_type_checking_blockhelper only checks forif TYPE_CHECKING:but won't matchif typing.TYPE_CHECKING:or aliased imports likefrom typing import TYPE_CHECKING as TC. This could lead to including type-checking-only imports in the output.src/designer_plugin/models.py (1)
108-129: Dataclass fields after fields with defaults require explicit defaults.
_traceback_strand_strhave default values ofNone, but if this dataclass is ever inherited or modified, the field ordering could cause issues. Also, capturing the stack trace in__post_init__means the trace will include dataclass internals rather than the actual error site.Consider whether capturing the traceback at exception creation is the desired behavior, or if it should be captured at raise time by the caller.
src/designer_plugin/api.py (2)
160-161: Usingprint()for output may not be appropriate in a library.Printing directly to stdout bypasses the logging system. Consider using
logger.info()or providing a callback mechanism for callers to handle output.
254-255: Duplicateprint()usage in sync path.Same concern as the async version - using
print()instead of logging.src/designer_plugin/d3sdk/client.py (1)
258-273: Potential IndexError when args exceed filtered_init_args.If more positional arguments are passed than there are
filtered_init_args, the code silently ignores the extra arguments. Consider adding validation or documentation about this behavior.src/designer_plugin/d3sdk/session.py (4)
67-82: Context manager entry registers modules but exit doesn't clean up on error.If module registration fails partway through (e.g., second of three modules), the first module remains registered on Designer but the context is never entered. Consider whether partial registration state needs cleanup or documentation.
84-86: Empty__exit__method could suppress exceptions if modified later.The method signature includes exception parameters but returns
None(implicit). This is correct, but adding an explicitreturn Noneorreturn Falsedocuments the intent to not suppress exceptions.
170-170: Accessing private class attribute_available_d3functionsfrom outside the class.Both session classes access
D3Function._available_d3functionsdirectly. Consider adding a public method toD3Functionlikeget_available_module_names()to expose this data.Also applies to: 314-314
300-318: Asyncregister_all_modulesregisters modules sequentially.For better performance with multiple modules, consider using
asyncio.gather()to register modules concurrently:async def register_all_modules( self, timeout_sec: float | None = None ) -> dict[str, bool]: modules: list[str] = list(D3Function._available_d3functions.keys()) results = await asyncio.gather( *[self.register_module(name, timeout_sec) for name in modules], return_exceptions=True ) return { name: isinstance(result, bool) and result for name, result in zip(modules, results) }src/designer_plugin/d3sdk/function.py (2)
285-300: Consider sorting packages and functions for deterministic registration payloads.
_available_packages[module_name]and_available_d3functions[module_name]are sets, socontentsordering is currently hash/order dependent. For reproducible payloads and more stable tests/debug logs, you may want deterministic ordering:- contents_packages: str = "\n".join( - list(D3Function._available_packages[module_name]) - ) - contents_functions: str = "\n\n".join( - [ - func.function_info.source_code_py27 - for func in D3Function._available_d3functions[module_name] - ] - ) + contents_packages: str = "\n".join( + sorted(D3Function._available_packages[module_name]) + ) + contents_functions: str = "\n\n".join( + func.function_info.source_code_py27 + for func in sorted( + D3Function._available_d3functions[module_name], + key=lambda f: f.name, + ) + )This keeps behavior the same while making
RegisterPayload.contentsstable across runs.
467-487: Make registry introspection helpers return results in a stable order.
get_all_d3functions()andget_all_modules()currently depend on dict/set iteration order, so their outputs can be non-deterministic. Sorting results makes these helpers nicer for tests, logging, and UIs:def get_all_d3functions() -> list[tuple[str, str]]: @@ - module_function_pairs: list[tuple[str, str]] = [] - for module_name, funcs in D3Function._available_d3functions.items(): - module_function_pairs += [ - (module_name, func.function_info.name) for func in funcs - ] - return module_function_pairs + module_function_pairs: list[tuple[str, str]] = [] + for module_name in sorted(D3Function._available_d3functions.keys()): + funcs = D3Function._available_d3functions[module_name] + module_function_pairs.extend( + (module_name, func.function_info.name) + for func in sorted(funcs, key=lambda f: f.name) + ) + return module_function_pairs @@ def get_all_modules() -> list[str]: @@ - return list(D3Function._available_d3functions.keys()) + return sorted(D3Function._available_d3functions.keys())This is behavior-preserving but improves predictability of the public API.
📜 Review details
Configuration used: CodeRabbit 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 (8)
README.md(2 hunks)src/designer_plugin/api.py(1 hunks)src/designer_plugin/d3sdk/ast_utils.py(1 hunks)src/designer_plugin/d3sdk/client.py(1 hunks)src/designer_plugin/d3sdk/function.py(1 hunks)src/designer_plugin/d3sdk/session.py(1 hunks)src/designer_plugin/models.py(1 hunks)tests/test_core.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/test_core.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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/models.pysrc/designer_plugin/d3sdk/function.pyREADME.md
🧬 Code graph analysis (4)
src/designer_plugin/api.py (4)
src/designer_plugin/models.py (9)
PluginError(92-97)PluginException(109-149)PluginPayload(154-172)PluginRegisterResponse(100-105)PluginResponse(45-89)RegisterPayload(175-186)debug_string(86-89)debug_string(167-172)debug_string(181-186)src/designer_plugin/d3sdk/session.py (2)
request(122-133)request(221-239)src/designer_plugin/d3sdk/client.py (1)
session(375-407)src/designer_plugin/d3sdk/function.py (2)
payload(198-211)payload(329-344)
src/designer_plugin/d3sdk/session.py (3)
src/designer_plugin/api.py (5)
Method(59-66)d3_api_arequest(97-123)d3_api_plugin(220-265)d3_api_register_module(268-311)d3_api_request(69-94)src/designer_plugin/d3sdk/function.py (5)
D3Function(214-344)module_name(303-309)payload(198-211)payload(329-344)get_module_register_payload(273-300)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)
src/designer_plugin/d3sdk/client.py (5)
src/designer_plugin/api.py (4)
d3_api_aplugin(128-171)d3_api_aregister_module(174-215)d3_api_plugin(220-265)d3_api_register_module(268-311)src/designer_plugin/d3sdk/ast_utils.py (5)
convert_class_to_py27(274-288)filter_base_classes(49-58)filter_init_args(61-79)get_class_node(31-44)get_source(15-28)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)src/designer_plugin/d3sdk/function.py (4)
module_name(303-309)payload(198-211)payload(329-344)name(159-165)src/designer_plugin/d3sdk/session.py (2)
register_module(135-156)register_module(275-298)
src/designer_plugin/d3sdk/function.py (2)
src/designer_plugin/d3sdk/ast_utils.py (2)
convert_function_to_py27(250-271)find_packages_in_current_file(293-382)src/designer_plugin/models.py (2)
PluginPayload(154-172)RegisterPayload(175-186)
🪛 markdownlint-cli2 (0.18.1)
README.md
94-94: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
180-180: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (8)
README.md (3)
85-85: Import structure is correct.All imports are properly sourced from correct modules:
TYPE_CHECKINGimports fromdesigner_plugin.pystub.d3are valid for IDE hints (separate PyPI package)D3PluginClient,d3pythonscript,d3function,D3AsyncSessioncorrectly imported fromdesigner_plugin.d3sdkPluginResponsecorrectly imported fromdesigner_plugin(main package re-export)D3Sessioncorrectly imported fromdesigner_plugin.d3sdkBased on learnings, the
designer-plugin-pystubpackage is installed separately via pip and provides type stubs for Designer's Python API.Also applies to: 100-100, 183-186, 220-220, 227-227
90-231: Documentation is comprehensive and well-structured.The Client API and Functional API sections clearly document:
- Sync/async execution patterns with practical examples
- Decorator semantics (
@d3pythonscriptvs.@d3function, registration requirements)- Session management and context manager usage
- Module grouping for function reuse and chaining
- Session API methods (
rpc()vs.execute())PluginResponsestructure with all fields documentedExamples are complete, practical, and demonstrate both synchronous and asynchronous patterns.
73-89: Stub and Logging sections are well-documented.The Stub file section properly documents the optional installation pattern and
TYPE_CHECKINGusage. The Logging section provides clear guidance for both quick debugging and production configuration, with appropriate recommendations against usingenable_debug_logging()in production.Also applies to: 234-251
src/designer_plugin/d3sdk/ast_utils.py (2)
15-28: Potential OSError propagation without handling.The docstring documents that
OSErrormay be raised if the source file cannot be found, butinspect.findsource()can also raiseOSErrorif the object was defined in C or if the source code cannot be retrieved. Consider whether callers expect this exception or if atry/exceptwith aNonereturn would be more appropriate for consistency with the return type.
31-44: LGTM!The function correctly uses
ast.walkto find the class node by name. Clean and straightforward implementation.src/designer_plugin/models.py (2)
59-69: LGTM!The
parse_returnValuevalidator now properly handles JSON decode errors by returning the raw string value, addressing the previous review feedback.
157-161: > Likely an incorrect or invalid review comment.src/designer_plugin/d3sdk/session.py (1)
45-176: LGTM on overall session architecture!The sync and async session classes provide a clean, consistent API. The separation of concerns with
D3SessionBaseand the use of context managers for session lifecycle management is well-designed.Also applies to: 178-319
3fe6492 to
9dca736
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: 3
🧹 Nitpick comments (3)
src/designer_plugin/d3sdk/ast_utils.py (1)
401-401: Misleading variable name:modulesshould bemodule.The variable is assigned a single module object (
types.ModuleType | None), not multiple modules. The plural name is confusing.Apply this diff:
- modules: types.ModuleType | None = inspect.getmodule(caller_frame) - if not modules: + module: types.ModuleType | None = inspect.getmodule(caller_frame) + if not module: return [] - source: str = inspect.getsource(modules) + source: str = inspect.getsource(module)README.md (1)
99-100: Optional: turn “Example” into proper headings (MD036)Static analysis flags the emphasized “Example” lines as heading-like; converting them to real headings improves structure and satisfies markdownlint.
-**Example** +### Example @@ -**Example** +### ExampleAlso applies to: 185-186
src/designer_plugin/d3sdk/client.py (1)
323-351: Docstring TYPE_CHECKING import should match the stub package and public docsThe
D3PluginClientusage example still imports stubs fromd3blobgen.scripts.d3, which doesn’t match the documented stub package (designer-plugin-pystub) and thedesigner_plugin.pystub.d3import pattern described in the README. Updating this keeps examples consistent and avoids confusion. Based on learnings, this stub package is installed separately and used only underTYPE_CHECKING.- from typing import TYPE_CHECKING - if TYPE_CHECKING: - from d3blobgen.scripts.d3 import * + from typing import TYPE_CHECKING + if TYPE_CHECKING: + from designer_plugin.pystub.d3 import *
📜 Review details
Configuration used: CodeRabbit 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)
README.md(2 hunks)pyproject.toml(4 hunks)src/designer_plugin/d3sdk/ast_utils.py(1 hunks)src/designer_plugin/d3sdk/client.py(1 hunks)src/designer_plugin/d3sdk/function.py(1 hunks)src/designer_plugin/models.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- pyproject.toml
- src/designer_plugin/d3sdk/function.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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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.mdsrc/designer_plugin/d3sdk/client.pysrc/designer_plugin/models.py
🪛 markdownlint-cli2 (0.18.1)
README.md
99-99: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
185-185: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (2)
src/designer_plugin/d3sdk/client.py (1)
161-270: Metaclass-based AST pipeline for remote clients looks coherentThe metaclass flow (source extraction, AST transforms, py27 conversion, instance_code template generation, and method wrapping) is internally consistent and matches the intended design described in the README. The wrapping correctly skips dunder methods while handling user-defined callables.
src/designer_plugin/models.py (1)
14-185: Model layer and returnValue parsing are well-structuredThe endpoint constants, PluginStatus/PluginResponse generics, safe
parse_returnValuehandling, and payload/debug helpers form a clear, robust contract for the API and SDK layers, matching the documented behavior (status + d3Log/pythonLog + typed returnValue).
9dca736 to
6674d83
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
♻️ Duplicate comments (4)
README.md (1)
224-224: Fix incorrect API reference in comment.The comment references "Use plugin()" but the code calls
session.execute(). Update the comment to match the actual API being demonstrated.- # Use plugin() for full response with logs and status + # Use execute() for full response with logs and statussrc/designer_plugin/d3sdk/client.py (3)
262-268: Comment/behavior mismatch: single-underscore methods are still wrapped.The comment says "Skip internal framework methods" but the condition
not attr_name.startswith("__")only skips dunder methods. Single-underscore methods like_helper()will be wrapped for remote execution.If single-underscore methods should remain local-only, update the condition:
# Wrap all user-defined public methods to execute remotely via Designer API - # Skip internal framework methods + # Skip private methods (_*) and dunder methods for attr_name, attr_value in attrs.items(): - if callable(attr_value) and not attr_name.startswith("__"): + if callable(attr_value) and not attr_name.startswith("_"): attrs[attr_name] = create_d3_plugin_method_wrapper( attr_name, attr_value )If wrapping single-underscore methods is intentional, update the comment to clarify.
286-302: PotentialKeyErrorwhen arguments don't match filtered parameters.If fewer positional args are passed than required by
filtered_init_args,arg_mappingwill have missing keys, causingformat(**arg_mapping)on line 302 to raise a confusingKeyError.Consider adding validation before formatting:
# Map keyword arguments that match filtered parameter names for key, value in kwargs.items(): if key in param_names: arg_mapping[key] = repr(value) + # Validate all required parameters are mapped + missing = set(param_names) - set(arg_mapping.keys()) + if missing: + raise TypeError(f"{cls.__name__}() missing required arguments: {missing}") + # Replace placeholders in template with actual serialized argument values instance_code: str = cls.instance_code_template.format(**arg_mapping)Also, the variable
filtered_idx = ion line 292 is unused—it's always equal toi. Consider removing the misleading comment and variable.
62-65: Module override is still not respected inbuild_payload.When a user calls
session(module_name="custom"), the module is registered under "custom" (via_get_register_module_payloadat line 475), butbuild_payloadstill usesself.module_name(the class name). This causes Designer to execute against the wrong module.Apply this diff to respect the session-level override:
# Build the Python script that will execute remotely on Designer script = f"return plugin.{method_name}({all_args})" # Create payload containing script and module info - return PluginPayload[Any](moduleName=self.module_name, script=script) + module_name = getattr(self, "_override_module_name", None) or self.module_name + return PluginPayload[Any](moduleName=module_name, script=script)
🧹 Nitpick comments (3)
README.md (1)
99-99: Convert bold text to proper markdown headings.Lines 99 and 185 use bold emphasis (
**Example**) where markdown headings should be used instead. This violates MD036 and reduces semantic clarity in the document structure.-**Example** +### ExampleAlso applies to: 185-185
src/designer_plugin/d3sdk/function.py (2)
53-121: Clarifyextract_function_infoerror semantics or wrap low‑level exceptionsThe docstring advertises
ValueErrorfor “not a function or cannot be parsed”, butinspect.getsource,ast.parse, andast.unparsecan also raiseOSError/TypeError/SyntaxErrordirectly. Either (a) wrap those inValueErrorfor a stable public contract, or (b) expand the docstring to reflect the actual exceptions that may bubble up.
239-312: Use deterministic ordering when building module registration contents
_available_packages[module_name]and_available_d3functions[module_name]are sets, so the join order of imports and function definitions is effectively arbitrary. This is fine for execution but makes payloads harder to diff or snapshot test.Consider sorting for stable output, e.g.:
- contents_packages: str = "\n".join( - list(D3Function._available_packages[module_name]) - ) - contents_functions: str = "\n\n".join( - [ - func.function_info.source_code_py27 - for func in D3Function._available_d3functions[module_name] - ] - ) + contents_packages: str = "\n".join( + sorted(D3Function._available_packages[module_name]) + ) + contents_functions: str = "\n\n".join( + func.function_info.source_code_py27 + for func in sorted( + D3Function._available_d3functions[module_name], + key=lambda f: f.name, + ) + )This keeps behavior the same while making register payloads predictable across runs.
📜 Review details
Configuration used: CodeRabbit 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)
README.md(2 hunks)pyproject.toml(3 hunks)src/designer_plugin/d3sdk/ast_utils.py(1 hunks)src/designer_plugin/d3sdk/client.py(1 hunks)src/designer_plugin/d3sdk/function.py(1 hunks)src/designer_plugin/models.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/designer_plugin/d3sdk/ast_utils.py
- src/designer_plugin/models.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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/d3sdk/client.pyREADME.mdsrc/designer_plugin/d3sdk/function.pypyproject.toml
🧬 Code graph analysis (1)
src/designer_plugin/d3sdk/function.py (2)
src/designer_plugin/d3sdk/ast_utils.py (4)
convert_function_to_py27(250-271)find_packages_in_current_file(366-455)validate_and_bind_signature(293-315)validate_and_extract_args(318-361)src/designer_plugin/models.py (2)
PluginPayload(154-172)RegisterPayload(175-186)
🪛 markdownlint-cli2 (0.18.1)
README.md
99-99: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
185-185: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (14)
pyproject.toml (4)
6-8: Project metadata updated appropriately for SDK release.The name change to "designer-plugin" and version bump to 1.2.0 align with the PR objectives and semantic versioning for new features. The description accurately reflects the new remote Python execution and DNS-SD discovery capabilities.
14-18: New dependencies correctly added for SDK functionality.The runtime dependencies align with the new SDK components:
aiohttpandrequestssupport synchronous and asynchronous HTTP operations (D3Session, D3AsyncSession)pydanticvalidates response models (PluginResponse, etc.)types-requestsprovides type hints for IDE supportThe flexible
>=version constraints are appropriate for a library. Based on learnings, ensure thetypes-requestsdate-tagged version (2032.4.20250913) is intentional for type stub stability.
26-26: License and type metadata properly configured.The MIT license field and
py.typedmarker are correctly added per PEP 561 and packaging guidelines. This resolves the past review comment and enables IDE autocomplete and type checking support—essential for the typed SDK.Also applies to: 28-29
36-36: Configuration and tooling updates are clean and consistent.
- Documentation URL points to the Designer Python execution API (useful reference for users).
twineadded to dev dependencies for PyPI publishing (aligns with designer-plugin and designer-plugin-pystub packages).- Directory references ("test" → "tests") are consistent across ruff and pytest configurations.
Also applies to: 46-46, 52-52, 105-105
src/designer_plugin/d3sdk/client.py (7)
1-40: LGTM!Imports are well-organized with clear separation between standard library, internal API, AST utilities, and models.
68-76: LGTM!Clear error message with usage example helps users understand the session requirement.
78-139: LGTM!Well-structured wrapper that correctly handles both async and sync methods, validates arguments, checks session state, and preserves original method metadata via
functools.wraps.
141-159: LGTM!Clean wrapper for payload-only generation without execution.
313-367: LGTM!The
D3PluginClientclass is well-documented with a clear usage example. Session state management with_hostname,_port, and_override_module_nameis properly implemented.
369-439: LGTM!Both
async_sessionandsessioncontext managers properly:
- Set the override module name when provided
- Clean up all session state in
finallyblocks- Handle registration conditionally
461-479: LGTM!
_get_register_module_payloadcorrectly uses_override_module_namewhen set, falling back to the class-levelmodule_name. This properly addresses the previous session-override concern for registration.src/designer_plugin/d3sdk/function.py (3)
128-224: Argument validation + payload generation looks solidThe combination of
inspect.signature,validate_and_extract_args, and repr‑based assignment/call generation correctly handles defaults, keyword‑only params, and*args/**kwargs, while surfacing signature mismatches asTypeError. The resulting script layout (assignments + body_py27) matches the intended “inline body” design for@d3pythonscript.
323-368: Call‑string generation with signature validation is appropriate
_args_to_stringvalidates against the function signature first and then usesreprfor both positional and keyword arguments, which is exactly what you want for generating a safe Python call expression in the payload. The separation of “inline body” vs “call by name in module” betweenD3PythonScript.payloadandD3Function.payloadis clean and easy to reason about.
450-511: Registry helpers (add_packages_in_current_file,get_*) are straightforward and cohesiveThe helpers cleanly encapsulate registry access and keep the public API surface simple (
get_register_payload,get_all_d3functions,get_all_modules). The use offind_packages_in_current_file(2)matches the documented “caller + decorator” call stack expectation.
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
🧹 Nitpick comments (4)
README.md (1)
99-100: Consider converting emphasized “Example” labels into headings or silencing MD036markdownlint flags the
**Example**lines as “emphasis used instead of a heading” (MD036). If you want a clean lint run, either change these to proper headings (e.g.,### Example) or disable MD036 for this file/section.Also applies to: 185-186
tests/test_client.py (3)
72-179: Strong coverage of sync signature validation; minor duplication could be reducedNice spread of tests around happy-path invocation and common
TypeErrorcases (too many positional args, multiple values, missing/extra args, defaults, keyword-only, mixed params). To keep this maintainable, consider a small helper or fixture that setsplugin._hostname/_portor enters aplugin.session(...)context, so each test focuses purely on the argument pattern under test.
172-178: Clarify the regex used for the missing keyword-only argument errorThe pattern
match="missing a required*"is a bit opaque because*acts as a quantifier on the letterd, not a wildcard. For readability and to future-proof across Python versions, consider a more explicit pattern, for example:with pytest.raises(TypeError, match=r"missing a required .*argument"): plugin.method_mixed(1, 2)This keeps the assertion flexible but clearer to the next reader.
403-459: Module-name override behavior is well covered; keep in mind private attribute couplingThe three tests here nicely pin down override semantics, default behavior, and cleanup on exception for
session(..., module_name=...), including verifying the actual registration payload. Note that asserting onplugin._override_module_namecouples tests to a private attribute; that’s fine for now but may need updating if internals change.
📜 Review details
Configuration used: CodeRabbit 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 (3)
README.md(2 hunks)src/designer_plugin/d3sdk/ast_utils.py(1 hunks)tests/test_client.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/designer_plugin/d3sdk/ast_utils.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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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.md
🧬 Code graph analysis (1)
tests/test_client.py (3)
src/designer_plugin/d3sdk/client.py (2)
in_session(361-367)session(406-439)src/designer_plugin/models.py (2)
PluginResponse(45-89)PluginStatus(30-37)src/designer_plugin/d3sdk/ast_utils.py (1)
validate_and_extract_args(318-368)
🪛 markdownlint-cli2 (0.18.1)
README.md
99-99: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
185-185: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🔇 Additional comments (2)
README.md (1)
80-94: Stub package usage and TYPE_CHECKING pattern look correctThe
designer-plugin-pystubinstall andif TYPE_CHECKING: from designer_plugin.pystub.d3 import *pattern cleanly separates runtime from IDE-only types and matches the documented stub-package setup. No changes needed here.Based on learnings, this correctly reflects the separate PyPI stub package design.
tests/test_client.py (1)
191-401: Comprehensive tests forvalidate_and_extract_argshelperThese cases exercise positional-only, keyword-only, defaults,
*args,**kwargs, mixed signatures, and “all parameter types” very thoroughly. The behavior of unpacking and preserving order is well specified, which should make future refactors ofvalidate_and_extract_argsmuch safer.
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
🧹 Nitpick comments (2)
tests/test_client.py (1)
172-178: Clarify the regex used for the missing-argument assertionThe pattern
match="missing a required*"is a bit opaque, since*here quantifies the finaldrather than acting as a wildcard. For readability and future-proofing, consider something like:with pytest.raises(TypeError, match=r"missing a required"): ... # or, if you want to allow suffix changes: with pytest.raises(TypeError, match=r"missing a required.*"): ...This makes the intent clearer without overfitting to the exact CPython wording.
src/designer_plugin/d3sdk/client.py (1)
323-347: Docstring import example may be out of date with the current stub packageThe usage example still references
from d3blobgen.scripts.d3 import *underTYPE_CHECKING. Given that the stubs have been moved to a separatedesigner-plugin-pystubpackage for IDE autocomplete, it might be less confusing to update this import to align with whatever module path that package exposes, or at least mention the stub package here instead ofd3blobgen.
Based on learnings, this would keep the docstring in sync with the recommended stub usage.
📜 Review details
Configuration used: CodeRabbit 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 (2)
src/designer_plugin/d3sdk/client.py(1 hunks)tests/test_client.py(1 hunks)
🧰 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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/d3sdk/client.py
🧬 Code graph analysis (2)
src/designer_plugin/d3sdk/client.py (5)
src/designer_plugin/api.py (4)
d3_api_aplugin(128-171)d3_api_aregister_module(174-214)d3_api_plugin(219-264)d3_api_register_module(267-310)src/designer_plugin/d3sdk/ast_utils.py (6)
convert_class_to_py27(274-288)filter_base_classes(49-58)filter_init_args(61-79)get_class_node(31-44)get_source(15-28)validate_and_extract_args(318-368)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)src/designer_plugin/d3sdk/function.py (4)
payload(207-223)payload(350-368)module_name(315-321)name(160-166)src/designer_plugin/d3sdk/session.py (2)
register_module(135-156)register_module(275-298)
tests/test_client.py (3)
src/designer_plugin/d3sdk/client.py (4)
in_session(361-367)session(406-439)_get_module_name(461-470)build_payload(42-65)src/designer_plugin/models.py (2)
PluginResponse(45-89)PluginStatus(30-37)src/designer_plugin/d3sdk/ast_utils.py (1)
validate_and_extract_args(318-368)
🔇 Additional comments (4)
tests/test_client.py (3)
15-62: SignatureValidationPlugin and fixtures give strong coverage of signature shapesThe helper plugin plus
plugin/mock_responsefixtures exercise a good variety of parameter kinds and success/error paths for wrapped methods; this looks solid.
191-401: Comprehensive coverage ofvalidate_and_extract_argsbehaviorThese tests cover positional, keyword-only, mixed, var-positional, var-keyword, and complex signatures, including defaults and error cases. This gives very good confidence in the helper’s correctness.
514-630:build_payloadtests nicely pin down script/moduleName behaviorThe payload tests probe positional/keyword/mixed args, string quoting, various types, nested structures, module-name overrides, and method-name preservation. This is a well-rounded test suite for the payload-building helper.
src/designer_plugin/d3sdk/client.py (1)
42-159: Remote-execution wrappers andbuild_payloadare consistent and well-factored
build_payloadnow correctly defers to_get_module_name()so session overrides are honored, and both async/sync wrappers share the same validation path viavalidate_and_extract_argsbefore delegating to the appropriate API (d3_api_aplugin/d3_api_plugin). The separation between execution wrappers and the payload-only wrapper keeps the responsibilities clear.
PeterSchuebel
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.
LGTM overall.
Maybe coderabbit has found some minor issues that you're still working on, so I'll wait with my approval until later.
| "License :: OSI Approved :: MIT License", | ||
| "Operating System :: OS Independent" | ||
| ] | ||
| readme = "README.md" |
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.
The license entry in "classifiers" could have remained, I think. According to the guide linked above it helps with searches on PyPi, e.g.: https://pypi.org/search/?c=License+%3A%3A+OSI+Approved+%3A%3A+MIT+License
Co-authored-by: Peter Schübel <git@peterschuebel.net>
src/designer_plugin/api.py
Outdated
| PluginException: If module registration fails on Designer side. | ||
| """ | ||
| try: | ||
| logger.debug(f"Register module:{payload.debug_string()}") |
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.
I think you should audit these logging messages - the f-string will be evaluated and then the result discarded unless debug logging is on. If you have f-string debug logging like this anywhere near a hot codepath, it'll cause nasty performance issues. I'd recommend setting a rule so f-strings are simply not allowed anywhere in any logging code, and instead we use the old printf-style formatting, which just takes a reference rather than building the entire string.
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.
Good shout, I'll hide them under isEnabledFor.
Thanks!
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.
This file feels like some utility you had for testing, rather than something we'd want in the actual API - python logging is python logging, and reaching in and mutating the logger config for the app isn't really something a library should be doing.
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.
I agree with you. I'll remove all but keep one helper get_logger for easy access.
LOGGER_NAME = "designer_plugin"
def get_logger():
return logging.getLogger(LOGGER_NAME)| D3_PLUGIN_ENDPOINT = "api/session/python/execute" | ||
| D3_PLUGIN_MODULE_REG_ENDPOINT = "api/session/python/registermodule" | ||
| D3_PLUGIN_DEFAULT_PORT = 80 |
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.
These don't seem to serve any particular purpose other than obfuscating what the URL is - it's only ever used once in the api.py file, so there's no reuse which would justify a variable
| from typing import Any, Generic, TypeVar | ||
|
|
||
| import typing_extensions | ||
| from pydantic import BaseModel, Field, TypeAdapter, field_validator |
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.
I'm not a huge fan of pydantic, personally - it does a lot more than deserialisation (pydantic model objects have a use-time cost, weirdly) and it brings in a fair number of dependencies for comparatively little work.
I didn't try it, but deserialisation of the objects might be simply:
json_obj = json.loads(json_data_str)
response = PluginResponse(**json_obj)
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.
I see your point, though I'm big fan of pydantic as they provide data validation in addition to de-serialisation.
For the network IO purpose, cost of pydantic is somewhat acceptable in my opinion.
twhittock-disguise
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.
I think it's probably worth getting this merged and then iterate. Some of my comments, if not addressed before merging, should probably get logged as GH issues.
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
♻️ Duplicate comments (2)
src/designer_plugin/d3sdk/client.py (2)
262-268: Clarify method wrapping comment to match behavior.The comment states "Skip private methods (
_*)" but the conditionnot attr_name.startswith("__")only skips dunder methods. Single-underscore methods (e.g.,_helper) are currently wrapped for remote execution.If this is intentional, update the comment to reflect the actual behavior. If single-underscore methods should remain local, change the condition to
not attr_name.startswith("_").
272-310: Default parameter handling causes KeyError for init with defaults.The
__call__method builds anarg_mappingdictionary and uses it to formatinstance_code_template. However, if a parameter has a default value and is not provided by the caller, it won't be inarg_mapping, causing.format(**arg_mapping)to raiseKeyError.Example:
class MyPlugin(D3PluginClient): def __init__(self, a, b=1): self.a = a self.b = b # This will fail with KeyError: 'b' plugin = MyPlugin(10)The template expects
{a}and{b}, butarg_mappingonly contains'a'.Consider building the instantiation code directly from provided args/kwargs, allowing the remote side to apply defaults:
- # Build mapping from parameter names to their repr() values for remote instantiation - param_names: list[str] = cls.filtered_init_args - arg_mapping: dict[str, str] = {} - - # Map positional arguments - for i, param_name in enumerate(param_names): - filtered_idx = i # Account for excluded client-side args - if filtered_idx < len(args): - arg_mapping[param_name] = repr(args[filtered_idx]) - - # Map keyword arguments that match filtered parameter names - for key, value in kwargs.items(): - if key in param_names: - arg_mapping[key] = repr(value) - - # Replace placeholders in template with actual serialized argument values - instance_code: str = cls.instance_code_template.format(**arg_mapping) + # Build argument list for remote instantiation, respecting defaults + param_names: list[str] = cls.filtered_init_args + arg_strings: list[str] = [] + + for i, param_name in enumerate(param_names): + if i < len(args): + arg_strings.append(repr(args[i])) + elif param_name in kwargs: + arg_strings.append(f"{param_name}={repr(kwargs[param_name])}") + # Skip parameters not provided - let remote side use defaults + + instance_code: str = f"plugin = {cls.__name__}({', '.join(arg_strings)})"
🧹 Nitpick comments (1)
src/designer_plugin/logger.py (1)
55-56: LGTM! Clean implementation following past feedback.The function correctly implements the simplified approach agreed upon in previous reviews. The implementation is straightforward with proper type hints.
Optional: Consider adding a brief function docstring for API documentation tools:
def get_logger() -> logging.Logger: + """Return the root logger for the designer_plugin package.""" return logging.getLogger(LOGGER_NAME)The module-level docstring already covers usage well, so this is purely for API doc generation tools like Sphinx or pdoc.
📜 Review details
Configuration used: CodeRabbit 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 (3)
src/designer_plugin/api.py(1 hunks)src/designer_plugin/d3sdk/client.py(1 hunks)src/designer_plugin/logger.py(1 hunks)
🧰 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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/api.pysrc/designer_plugin/d3sdk/client.py
🧬 Code graph analysis (2)
src/designer_plugin/api.py (4)
src/designer_plugin/models.py (7)
PluginPayload(154-172)PluginRegisterResponse(100-105)PluginResponse(45-89)RegisterPayload(175-186)debug_string(86-89)debug_string(167-172)debug_string(181-186)src/designer_plugin/d3sdk/session.py (2)
request(122-133)request(221-239)src/designer_plugin/d3sdk/client.py (1)
session(408-443)src/designer_plugin/d3sdk/function.py (2)
payload(207-223)payload(350-368)
src/designer_plugin/d3sdk/client.py (5)
src/designer_plugin/api.py (4)
d3_api_aplugin(128-173)d3_api_aregister_module(176-217)d3_api_plugin(222-269)d3_api_register_module(272-316)src/designer_plugin/d3sdk/ast_utils.py (3)
convert_class_to_py27(274-288)filter_init_args(61-79)get_class_node(31-44)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)src/designer_plugin/d3sdk/function.py (4)
payload(207-223)payload(350-368)module_name(315-321)name(160-166)src/designer_plugin/d3sdk/session.py (2)
register_module(135-156)register_module(275-298)
🔇 Additional comments (6)
src/designer_plugin/logger.py (2)
1-47: LGTM! Comprehensive and helpful documentation.The module docstring provides excellent coverage of logging configuration patterns, from basic setup to advanced granular control. The examples are clear and the hierarchical logging explanation will help users troubleshoot specific components effectively.
49-52: LGTM! Clean constant definition.The logger name correctly matches the package name and follows standard Python conventions.
src/designer_plugin/api.py (1)
1-316: LGTM! Clean HTTP API layer implementation.The API layer provides well-structured synchronous and asynchronous wrappers for Designer plugin interactions. Error handling is correctly centralized at the
PluginResponse.parse_returnValuelayer, and all debug logging with f-strings is properly guarded withisEnabledFor(logging.DEBUG)checks to avoid performance issues.src/designer_plugin/d3sdk/client.py (3)
42-66: LGTM! Payload building correctly handles module override.The
build_payloadfunction properly serializes arguments and uses_get_module_name()to respect session-level module overrides.
78-138: LGTM! Method wrapper correctly handles async/sync methods.The wrapper implementation properly validates arguments, checks session state, and delegates to the appropriate Designer API functions while preserving original method metadata.
313-493: LGTM! D3PluginClient base class is well-structured.The base class properly manages session state, handles module name overrides via
_override_module_name, and provides both sync and async context managers for Designer sessions. The module registration flow correctly uses_get_module_name()to respect session-level overrides.
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
🧹 Nitpick comments (4)
tests/test_client.py (1)
47-180: Signature validation tests are thorough; minor regex nitThe
TestSignatureValidationcases give good coverage of argument-shape errors and success paths, including the “no session” guard. One small nit: intest_mixed_parameters_missing_keyword_only, the regexmatch="missing a required*"is slightly misleading—*only applies to the finald, so this behaves the same as matching"missing a required". If you wanted a flexible suffix, considermatch="missing a required.*", or just"missing a required argument"for clarity.src/designer_plugin/d3sdk/client.py (3)
141-158: Consider reusing signature validation increate_d3_payload_wrapper
create_d3_payload_wrappercurrently bypassesvalidate_and_extract_args, so payload-only wrappers can be called with argument shapes that would be rejected by the regular RPC wrappers. For consistency (and earlier, clearer errors), consider runningvalidate_and_extract_argshere too and then passing its(positional, keyword)result intobuild_payload, mirroringcreate_d3_plugin_method_wrapper.
206-208: Comment and behavior aroundmodule_namedefaulting are slightly out of syncThe comment says “Use class name as default module_name if not explicitly provided”, but
attrs["module_name"] = nameoverwrites anymodule_namea subclass might define. Either respect an existingmodule_name(e.g., only set it if not inattrs) or update the comment to reflect that the class name always wins.
312-343: Docstring example import path may be outdated vs the stub packageThe example still references
from d3blobgen.scripts.d3 import *underTYPE_CHECKING, whereas the documented stub package isdesigner-plugin-pystuband is meant to be used for IDE autocomplete in this project. Consider updating this snippet to import from the stub package so that users copying the example get a consistent setup with the README.
📜 Review details
Configuration used: CodeRabbit 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 (2)
src/designer_plugin/d3sdk/client.py(1 hunks)tests/test_client.py(1 hunks)
🧰 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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/d3sdk/client.py
🧬 Code graph analysis (1)
src/designer_plugin/d3sdk/client.py (5)
src/designer_plugin/api.py (4)
d3_api_aplugin(128-173)d3_api_aregister_module(176-217)d3_api_plugin(222-269)d3_api_register_module(272-316)src/designer_plugin/d3sdk/ast_utils.py (6)
convert_class_to_py27(274-288)filter_base_classes(49-58)filter_init_args(61-79)get_class_node(31-44)get_source(15-28)validate_and_extract_args(318-368)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)src/designer_plugin/d3sdk/function.py (4)
payload(207-223)payload(350-368)module_name(315-321)name(160-166)src/designer_plugin/d3sdk/session.py (2)
register_module(135-156)register_module(275-298)
🔇 Additional comments (6)
tests/test_client.py (5)
15-45: Helper plugin nicely exercises a wide range of signatures
SignatureValidationPlugincovers required, defaulted, positional-only, keyword-only, mixed, and async methods, which is ideal for driving metaclass and wrapper behavior in tests. No issues here.
191-401: Excellent coverage ofvalidate_and_extract_argsedge casesThe
TestValidateAndExtractArgssuite exercises positional, keyword-only, defaults, var-positional, var-keyword, and complex mixed signatures and ensures ordering is preserved. This should make regressions invalidate_and_extract_argsvery hard to introduce. No changes needed.
403-512: Module-name override behavior is well specified by testsThe
TestModuleNameOverridetests clearly pin down how_override_module_nameinteracts with sessions and_get_module_name, including cleanup on normal exit and exceptions. This matches the intended semantics for per-session overrides and should prevent regressions. Looks good.
514-631: Payload-building tests capture key script-generation scenarios
TestBuildPayloaddoes a good job verifying moduleName selection (including overrides) and the generatedscriptacross positional, keyword, mixed, and nested-structure arguments. The reliance onrepr()formatting is reasonable and aligns with how the client works. No issues spotted.
633-895: Instance-code generation tests robustly validate the metaclass fix
TestInstanceCodeGenerationWithDefaultsthoroughly exercisesD3PluginClientMeta.__call__with combinations of required/optional params, positional vs keyword arguments, and different default types. The expectations (letting the remote side apply defaults, only emitting provided args/overrides) align well with the new implementation and should guard against regressions ininstance_codegeneration.src/designer_plugin/d3sdk/client.py (1)
434-484: Module registration helpers correctly honor session overrides
_get_module_name,_get_register_module_content, and_get_register_module_payloadwork together to ensure that registration uses any session-level override and includes the Python 2.7-converted class plusinstance_code. This matches the behavior exercised in the tests and earlier review comments about module-name overrides. No further changes needed here.
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
♻️ Duplicate comments (1)
CONTRIBUTING.md (1)
252-252: Hyphenate “user‑facing” in documentation bullet.For consistency with standard compound adjectives and prior review, change “user facing” → “user‑facing”.
-- Update README.md for user facing changes +- Update README.md for user-facing changes
🧹 Nitpick comments (1)
tests/test_client.py (1)
522-576: Align “no arguments” payload test with the target method’s signature.
test_build_payload_with_no_argumentsdescribes a method that “takes no arguments” but uses"simple_method"which requiresaandb. The payload built here (return plugin.simple_method()) would fail at remote execution if actually used.Either adjust the test to use a truly no‑arg method, or update the docstring/name to reflect that this is intentionally exercising the “zero args provided” case for a method that normally requires parameters.
📜 Review details
Configuration used: CodeRabbit 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 (4)
CONTRIBUTING.md(1 hunks)src/designer_plugin/d3sdk/client.py(1 hunks)src/designer_plugin/logger.py(1 hunks)tests/test_client.py(1 hunks)
🧰 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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/d3sdk/client.pyCONTRIBUTING.md
🧬 Code graph analysis (1)
src/designer_plugin/d3sdk/client.py (4)
src/designer_plugin/d3sdk/ast_utils.py (5)
convert_class_to_py27(274-288)filter_base_classes(49-58)filter_init_args(61-79)get_class_node(31-44)get_source(15-28)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)src/designer_plugin/d3sdk/function.py (4)
payload(207-223)payload(350-368)module_name(315-321)name(160-166)src/designer_plugin/d3sdk/session.py (2)
register_module(135-156)register_module(275-298)
🪛 LanguageTool
CONTRIBUTING.md
[style] ~78-~78: Try moving the adverb to make the sentence clearer.
Context: ... Pre-commit Hooks Pre-commit hooks are configured to automatically run: - ruff check --fix - Linting with au...
(SPLIT_INFINITIVE)
[grammar] ~252-~252: Use a hyphen to join words.
Context: ...cumentation - Update README.md for user facing changes - Update docstrings for A...
(QB_NEW_EN_HYPHEN)
🔇 Additional comments (5)
src/designer_plugin/logger.py (1)
1-56: Minimal logger helper API looks good.The module now cleanly exposes just
LOGGER_NAMEandget_logger()without mutating global logging configuration, which aligns with standard logging practices and earlier review feedback.src/designer_plugin/d3sdk/client.py (2)
42-159: Payload building and wrappers are cohesive and respect argument validation.
build_payloadcentralizes script construction, and both async/sync wrappers correctly delegate tovalidate_and_extract_args, enforcein_session(), and route through the appropriated3_api_*function while preserving metadata viafunctools.wraps. Using_get_module_name()here correctly honors session-level module overrides.
374-511: Session management and module‑name override handling look solid.
async_session/sessioncorrectly set and clear_hostname,_port, and_override_module_namein atry/finally, and_get_module_name()is used consistently in both payload building and registration so per‑session overrides behave as documented._get_register_module_content()neatly stitches together the Py2.7 class definition andinstance_code, matching the design of remoteplugininstantiation.tests/test_client.py (2)
15-189: Strong coverage for wrapped method signature validation.The
SignatureValidationPluginplusTestSignatureValidation/TestValidateAndExtractArgssuites thoroughly cover positional, keyword-only, varargs/kwargs, defaults, and error cases, matching the behavior ofvalidate_and_extract_argsand the remote wrappers.
633-1013: Instance‑code and init‑wrapper tests are comprehensive and match the metaclass behavior.The
TestInstanceCodeGenerationWithDefaultsandTestInitWrapperSuperCallsuites cover a wide range of__init__signatures (required, defaults, positional/keyword mixes) and super‑call patterns, ensuringinstance_codegeneration and automatic base initialisation behave correctly across realistic plugin definitions.
24f418c to
9a9c3a2
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: 1
🧹 Nitpick comments (1)
src/designer_plugin/d3sdk/client.py (1)
282-288: Comment/code mismatch: single-underscore methods are still wrapped.The comment says "Skip internal framework methods" but the condition
not attr_name.startswith("__")only skips dunder methods. Single-underscore methods like_helper()will be wrapped for remote execution.If this is intentional (exposing
_methodas remotely callable), update the comment. If single-underscore methods should remain local, update the condition.Option 1 - Update comment to match behavior:
- # Wrap all user-defined public methods to execute remotely via Designer API - # Skip internal framework methods + # Wrap all user-defined methods to execute remotely via Designer API + # Skip dunder methods (__init__, __str__, etc.) for attr_name, attr_value in attrs.items(): if callable(attr_value) and not attr_name.startswith("__"):Option 2 - Skip single-underscore methods:
for attr_name, attr_value in attrs.items(): - if callable(attr_value) and not attr_name.startswith("__"): + if callable(attr_value) and not attr_name.startswith("_"):
📜 Review details
Configuration used: CodeRabbit 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 (2)
src/designer_plugin/d3sdk/client.py(1 hunks)tests/test_client.py(1 hunks)
🧰 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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/d3sdk/client.py
🧬 Code graph analysis (2)
tests/test_client.py (3)
src/designer_plugin/d3sdk/client.py (5)
D3PluginClient(336-516)in_session(384-390)session(431-466)_get_module_name(488-497)build_payload(42-65)src/designer_plugin/models.py (2)
PluginResponse(45-89)PluginStatus(30-37)src/designer_plugin/d3sdk/ast_utils.py (1)
validate_and_extract_args(318-368)
src/designer_plugin/d3sdk/client.py (4)
src/designer_plugin/d3sdk/ast_utils.py (6)
convert_class_to_py27(274-288)filter_base_classes(49-58)filter_init_args(61-79)get_class_node(31-44)get_source(15-28)validate_and_extract_args(318-368)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)src/designer_plugin/d3sdk/function.py (4)
payload(207-223)payload(350-368)module_name(315-321)name(160-166)src/designer_plugin/d3sdk/session.py (2)
register_module(135-156)register_module(275-298)
🪛 GitHub Actions: CI
src/designer_plugin/d3sdk/client.py
[error] 311-311: Unused "type: ignore" comment [unused-ignore]
🔇 Additional comments (22)
tests/test_client.py (9)
1-14: LGTM!License header and imports are correctly structured for the test module.
15-45: LGTM!Well-designed test plugin class covering all parameter kinds: positional, keyword, defaults, positional-only, keyword-only, mixed, and async. Provides comprehensive coverage for signature validation tests.
47-189: LGTM!Comprehensive test coverage for signature validation including session checks, argument validation errors, default parameters, positional-only, keyword-only, and mixed parameter types.
191-401: LGTM!Thorough test coverage for argument extraction utility covering all parameter kinds including
*args,**kwargs, positional-only, keyword-only, and complex mixed signatures.
403-511: LGTM!Excellent coverage of module name override functionality including session lifecycle, exception cleanup, and
_get_module_name()behavior in various contexts.
514-631: LGTM!Comprehensive payload building tests covering no arguments, positional, keyword, mixed, string escaping, various types, nested structures, and module name overrides.
633-896: LGTM!Thorough test coverage for instance code generation with default parameters, ensuring the remote side applies defaults rather than hardcoding them client-side. Good documentation explaining the issue being addressed.
898-979: LGTM!Good coverage of the metaclass
__call__guard, ensuring baseD3PluginClientcan be instantiated withoutAttributeErrorand that instrumentation attributes are only present on subclasses.
981-1096: LGTM!Excellent coverage of the
__init__wrapper ensuring required attributes are initialized even when users forget to callsuper().__init__(), without causing double initialization when they do call it.src/designer_plugin/d3sdk/client.py (13)
1-40: LGTM!Clean license header, imports, and type variable definitions.
42-66: LGTM!
build_payloadnow correctly usesself._get_module_name()to respect session-level module name overrides, addressing the previously raised issue.
68-76: LGTM!Clear, helpful error message for out-of-session method calls with usage example.
78-139: LGTM!Well-documented wrapper that properly:
- Validates arguments against the original signature
- Checks session state before execution
- Handles both async and sync methods
- Preserves function metadata via
functools.wraps
141-159: LGTM!Simple payload wrapper for generating payloads without executing remotely.
161-183: LGTM!This wrapper addresses the past review concern about users forgetting to call
super().__init__(). It ensures required attributes are always initialized by callingD3PluginClient.__init__(self)before the user's__init__.
185-224: LGTM!Comprehensive docstring explaining the metaclass transformations: source extraction, code filtering, Python 2.7 conversion, method wrapping, and code generation.
225-290: LGTM!The
__new__method properly:
- Skips instrumentation for the base class
- Extracts and transforms source code via AST
- Wraps
__init__to ensure parent initialization- Wraps public methods for remote execution
305-333: LGTM!The
__call__method properly:
- Guards against non-instrumented classes (base class check at line 307-308)
- Builds argument strings directly from provided args/kwargs, avoiding KeyError
- Allows remote side to apply defaults by not including unprovided parameters
336-377: LGTM!Clear docstring with accurate usage example showing sync context manager usage.
379-391: LGTM!Simple
__init__initializing session state attributes. Thein_session()method works correctly becausecreate_init_wrapperensures these attributes are always initialized.
392-467: LGTM!Both async and sync session context managers properly:
- Set module name override if provided
- Set hostname/port for active session
- Optionally register the module
- Clean up all session state in
finallyblock
468-516: LGTM!Registration helpers properly use
_get_module_name()to respect session overrides and construct the complete module content from Python 2.7 source code and instance code.
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
♻️ Duplicate comments (1)
src/designer_plugin/d3sdk/client.py (1)
282-288: Comment still doesn't match behavior for single-underscore methods.The condition
not attr_name.startswith("__")only skips dunder methods, but single-underscore methods like_helper()will still be wrapped for remote execution. The comment should be updated to match the behavior, or the condition should skip single-underscore methods if they're meant to stay local.
🧹 Nitpick comments (2)
tests/test_client.py (1)
419-424: Consider using named attribute access for mock call_args.The index-based access
call_args[0][2]is fragile if the API signature changes. Usingcall_args.args[2]or keyword argument access would be clearer:mock_register.assert_called_once() call_args = mock_register.call_args - payload = call_args[0][2] # Third positional argument is the payload + payload = call_args.args[2] # Third positional argument is the payloadOr consider capturing the call with a mock side effect that stores the payload for inspection.
src/designer_plugin/d3sdk/client.py (1)
499-505: Consider defensive attribute access for registration content.
_get_register_module_contentassumessource_code_py27andinstance_codeexist, which is only true for instrumented subclasses. While registration is unlikely to be called on the base class, a guard could make this more robust:def _get_register_module_content(self) -> str: """Generate the complete module content to register with Designer.""" + if not hasattr(self, "source_code_py27") or not hasattr(self, "instance_code"): + raise RuntimeError( + f"{self.__class__.__name__} is not instrumented for remote execution" + ) return f"{self.source_code_py27}\n\n{self.instance_code}"
📜 Review details
Configuration used: CodeRabbit 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 (2)
src/designer_plugin/d3sdk/client.py(1 hunks)tests/test_client.py(1 hunks)
🧰 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.173Z
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.173Z
Learnt from: DevTGHa
Repo: disguise-one/python-plugin PR: 4
File: README.md:97-97
Timestamp: 2025-12-01T16:07:14.173Z
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:
src/designer_plugin/d3sdk/client.py
🧬 Code graph analysis (2)
tests/test_client.py (2)
src/designer_plugin/d3sdk/client.py (4)
in_session(384-390)session(431-466)_get_module_name(488-497)build_payload(42-65)src/designer_plugin/d3sdk/ast_utils.py (1)
validate_and_extract_args(318-368)
src/designer_plugin/d3sdk/client.py (5)
src/designer_plugin/api.py (4)
d3_api_aplugin(128-173)d3_api_aregister_module(176-217)d3_api_plugin(222-269)d3_api_register_module(272-316)src/designer_plugin/d3sdk/ast_utils.py (6)
convert_class_to_py27(274-288)filter_base_classes(49-58)filter_init_args(61-79)get_class_node(31-44)get_source(15-28)validate_and_extract_args(318-368)src/designer_plugin/models.py (3)
PluginPayload(154-172)PluginResponse(45-89)RegisterPayload(175-186)src/designer_plugin/d3sdk/function.py (4)
payload(207-223)payload(350-368)module_name(315-321)name(160-166)src/designer_plugin/d3sdk/session.py (2)
register_module(135-156)register_module(275-298)
🔇 Additional comments (12)
tests/test_client.py (6)
1-45: LGTM! Well-structured test plugin with comprehensive method signatures.The
SignatureValidationPluginclass provides excellent coverage of different parameter types including positional-only, keyword-only, mixed parameters, defaults, and async methods.
47-189: Comprehensive signature validation tests covering sync/async paths.Good coverage of error cases including session validation, argument count mismatches, and parameter kind handling.
191-401: Thorough tests for argument extraction utility.Excellent coverage of all parameter kinds including
VAR_POSITIONAL,VAR_KEYWORD, positional-only, and keyword-only parameters. The tests correctly verify that arguments are separated back into positional and keyword arguments for remote execution.
514-631: Good coverage of payload building scenarios.Tests correctly verify:
- Argument serialization using
repr()- Module name override behavior
- Complex nested structures handling
- Method name preservation in scripts
633-896: Excellent tests for instance_code generation with defaults.These tests comprehensively address the issue where
D3PluginClientMeta.__call__could fail with KeyError. Good verification that:
- Omitted optional args rely on remote-side defaults (line 843-844)
- Keyword arguments are handled correctly
- Complex scenarios with mixed required/optional args work
898-1096: Solid tests for metaclass guards and init wrapper.
TestMetaclassCallGuardcorrectly verifies that baseD3PluginClientcan be instantiated withoutAttributeError, andTestInitWrapperSuperCallensures the framework handles subclasses that forget to callsuper().__init__().src/designer_plugin/d3sdk/client.py (6)
42-65: LGTM! Payload building correctly respects session-level module name override.The implementation properly uses
self._get_module_name()to respect any session-level module name override.
78-138: Well-implemented method wrapper with proper async/sync handling.The wrapper correctly:
- Validates arguments against the original signature
- Checks for active session before making API calls
- Uses the appropriate async/sync API based on the original method
161-182: LGTM! Safe init wrapper ensures base attributes are always initialized.The wrapper correctly ensures
_hostname,_port, and_override_module_nameare initialized even when subclasses omitsuper().__init__(). The base__init__is idempotent, so no harm if called twice.
305-333: LGTM! Robust instance code generation with proper guard for base class.The guard at lines 307-308 prevents
AttributeErrorwhen instantiating the baseD3PluginClientdirectly. The arg string building correctly respects defaults by omitting unprovided arguments.
336-391: LGTM! Base class provides clean session management foundation.The
__init__properly initializes session state, andin_session()provides a straightforward check. The init wrapper ensures these attributes exist even when subclasses omitsuper().__init__().
392-466: LGTM! Session context managers with proper cleanup in finally blocks.Both sync and async session managers correctly:
- Set override module name if provided
- Configure hostname/port for session duration
- Register module when requested
- Clean up all session state in finally blocks (even on exceptions)
Add d3sdk
Important Note
I have published the
d3.pyistub file on PyPI, so users can install it directly via pip:For further details:
Summary
This PR adds
d3sdkas a sub-package ofdesigner_plugin.Main APIs
D3ClientPluginsessionandasync_sessionfor module registration.d3pythonscriptpayloadmethod to retrieve JSON payloads for the execute endpoint.d3functionpayloadmethod to retrieve JSON payloads for the execute endpoint, with registration support.D3Session/D3AsyncSessionUpdated README.md
designer-plugin
A Python library for creating and managing plugins for Disguise Designer. This library provides:
Installation
To install the plugin, use pip:
Publish Plugin
The
DesignerPluginclass allows you to publish a plugin for the Disguise Designer application. Theportparameter corresponds to an HTTP server that serves the plugin's web user interface. Below is an example of how to use it (without a server, for clarity).In the working directory for the plugin (usually next to the plugin script) a
d3plugin.jsonshould be created. See The developer documentation for more information{ "name": "MyPlugin", "requiresSession": true }The script may work with
asyncioor be synchronous - both options are shown in this example:Publish options
If you would prefer not to use the
d3plugin.jsonfile, construct theDesignerPluginobject directly. The plugin's name and port number are required parameters. Optionally, the plugin can specifyhostname, which can be used to direct Designer to a specific hostname when opening the plugin's web UI, and other metadata parameters are available, also.Execute Python
Python scripts can be executed remotely on Designer via the plugin system.
Direct interaction with the plugin API endpoint requires extensive boilerplate code and JSON parsing. However, the Client API and Functional API simplify this process by providing an RPC (Remote Procedure Call) interface that abstracts away the underlying HTTP communication and payload management.
Stub file
To enable IDE autocomplete and type checking for Designer's Python API, install the stub file package:
Once installed, import the stubs using the
TYPE_CHECKINGpattern. This provides type hints in your IDE without affecting runtime execution:This allows you to get autocomplete for Designer objects like
resourceManager,Screen2,Path, etc., while writing your plugin code.Client API
The Client API allows you to define a class with methods that execute remotely on Designer by simply inheriting from
D3PluginClient. The Client API supports both async and sync methods.Example
Functional API
The Functional API provides finer control over remote execution compared to the Client API. While the Client API automatically manages the entire execution lifecycle (registration and execution are transparent), the Functional API gives you explicit control over:
payload()method to generate execution payloadssession.execute()for full response (status, logs, return value) orsession.rpc()for just the return valueThe Functional API offers two decorators:
@d3pythonscriptand@d3function:@d3pythonscript:@d3function:module_nameare grouped together and can call each other, enabling function chaining and code reuse.D3AsyncSession('localhost', 80, ["mymodule"])). If you don't provide module names, no registration occurs.Session API Methods
Both
D3AsyncSessionandD3Sessionprovide two methods for executing functions:session.rpc(payload)- Returns only the return value from the function execution. Simpler for most use cases.session.execute(payload)- Returns aPluginResponseobject containing:returnValue: The function's return valuestatus: Execution status (code, message, details)d3Log: Designer console output during executionpythonLog: Python-specific output (print statements, warnings)Example
Logging
By default,
designer_pluginlogging is disabled. To enable it:For production, use your application's logging configuration instead of
enable_debug_logging().License
This project is licensed under the MIT License. See the LICENSE file for details.
Summary by CodeRabbit
New Features
Documentation
Chores
Tests
✏️ Tip: You can customize this high-level summary in your review settings.