Conversation
Introduces a top-level init(address, options) function that creates an III client and auto-starts its connection task, matching the existing Node SDK pattern. Updates examples to use init() instead of manual III::new + connect.
Aligns the main SDK README with the new init() convenience function for Python and Rust.
📝 WalkthroughWalkthroughPython and Rust SDKs replace direct III construction with a new init(address, InitOptions) API that returns an III instance and starts connection work in the background; examples, tests, docs, and example app code updated to use InitOptions and injected III instances. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application
participant Init as init(address, InitOptions)
participant III as III Instance
participant Runtime as Async Runtime
participant Conn as Background Connect Task
App->>Init: call init(address, InitOptions)
Init->>III: create III (apply metadata/otel)
Init->>Runtime: schedule/spawn connect() task
Init-->>App: return III instance
Runtime->>Conn: run connect task
Conn->>III: iii.connect()
Conn-->>Runtime: log result / finish
App->>III: register functions / make calls
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
sdk/packages/rust/iii/tests/init_api.rs (1)
23-39: Test name overstates what is asserted.Line 23 says OTEL is applied before auto-connect, but the test only verifies successful init + registration. Consider either renaming the test or asserting observable OTEL side effects.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/rust/iii/tests/init_api.rs` around lines 23 - 39, The test name init_applies_otel_config_before_auto_connect overclaims the behavior; either rename the test to accurately reflect what's asserted (e.g., init_applies_otel_config_and_allows_registration or init_sets_otel_config_and_registers) or add an assertion that verifies an OTEL side effect (for example, inspect the configured Otel/tracer provider after calling init, or assert that no auto-connection occurred before registration) — update the test function init_applies_otel_config_before_auto_connect and its use of InitOptions/OtelConfig and the created client (client.register_function) accordingly to implement the chosen fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@sdk/packages/python/iii-example/src/state.py`:
- Line 3: The State.set method uses iii.call() without initializing iii, causing
a NameError; fix it by calling get_iii() and assigning it to iii at the start of
State.set (matching the pattern used in other methods), i.e., add iii =
get_iii() before any use of iii.call() so State.set initializes the III client
before invoking its call method.
---
Nitpick comments:
In `@sdk/packages/rust/iii/tests/init_api.rs`:
- Around line 23-39: The test name init_applies_otel_config_before_auto_connect
overclaims the behavior; either rename the test to accurately reflect what's
asserted (e.g., init_applies_otel_config_and_allows_registration or
init_sets_otel_config_and_registers) or add an assertion that verifies an OTEL
side effect (for example, inspect the configured Otel/tracer provider after
calling init, or assert that no auto-connection occurred before registration) —
update the test function init_applies_otel_config_before_auto_connect and its
use of InitOptions/OtelConfig and the created client (client.register_function)
accordingly to implement the chosen fix.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
sdk/packages/python/iii-example/uv.lockis excluded by!**/*.locksdk/packages/rust/iii-example/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
sdk/README.mdsdk/packages/python/iii-example/config.yamlsdk/packages/python/iii-example/src/hooks.pysdk/packages/python/iii-example/src/iii.pysdk/packages/python/iii-example/src/main.pysdk/packages/python/iii-example/src/state.pysdk/packages/python/iii-example/src/stream.pysdk/packages/python/iii/README.mdsdk/packages/python/iii/src/iii/__init__.pysdk/packages/python/iii/src/iii/iii.pysdk/packages/python/iii/tests/test_init_api.pysdk/packages/rust/iii-example/src/main.rssdk/packages/rust/iii/README.mdsdk/packages/rust/iii/src/error.rssdk/packages/rust/iii/src/lib.rssdk/packages/rust/iii/tests/init_api.rs
There was a problem hiding this comment.
🧹 Nitpick comments (2)
sdk/packages/python/iii-example/src/main.py (2)
13-14: Consider using specific types instead ofAnyfor module-level variables.Using
Anyforstateandstreamsloses type safety and IDE support. Consider using the actual types withNoneunion:♻️ Suggested type improvement
-state: Any = None -streams: Any = None +from .state import State +from .stream import StreamClient + +state: State | None = None +streams: StreamClient | None = NoneNote: If circular imports are a concern with top-level imports, you could use
TYPE_CHECKING:from typing import TYPE_CHECKING if TYPE_CHECKING: from .state import State from .stream import StreamClient state: "State | None" = None streams: "StreamClient | None" = None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/python/iii-example/src/main.py` around lines 13 - 14, Replace the module-level Any annotations for state and streams with their actual types to restore type safety: use the concrete classes (e.g. State and StreamClient) in the annotations for state and streams respectively, or if importing them at top-level causes circular imports, use typing.TYPE_CHECKING and forward-reference string annotations (e.g. "State | None", "StreamClient | None") so the variables are typed as optional instances rather than Any; update the declarations for state and streams accordingly and import types only under TYPE_CHECKING if needed.
22-22: Add type annotation foriiiparameter.For consistency with the rest of the codebase where
iii: IIIis explicitly typed, consider adding the type annotation here.♻️ Suggested fix
+from iii import III + -def _setup(iii) -> None: +def _setup(iii: III) -> None:Note: You'll need to add
IIIto the import on line 11.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@sdk/packages/python/iii-example/src/main.py` at line 22, The _setup function is missing a type annotation for its parameter; update the signature of _setup to declare the parameter as iii: III (i.e., def _setup(iii: III) -> None:) and add III to the imports mentioned on line 11 so the type is available for annotation; ensure you import III from the same module used elsewhere in the file to keep types consistent with the codebase.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@sdk/packages/python/iii-example/src/main.py`:
- Around line 13-14: Replace the module-level Any annotations for state and
streams with their actual types to restore type safety: use the concrete classes
(e.g. State and StreamClient) in the annotations for state and streams
respectively, or if importing them at top-level causes circular imports, use
typing.TYPE_CHECKING and forward-reference string annotations (e.g. "State |
None", "StreamClient | None") so the variables are typed as optional instances
rather than Any; update the declarations for state and streams accordingly and
import types only under TYPE_CHECKING if needed.
- Line 22: The _setup function is missing a type annotation for its parameter;
update the signature of _setup to declare the parameter as iii: III (i.e., def
_setup(iii: III) -> None:) and add III to the imports mentioned on line 11 so
the type is available for annotation; ensure you import III from the same module
used elsewhere in the file to keep types consistent with the codebase.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
sdk/packages/python/iii-example/src/hooks.pysdk/packages/python/iii-example/src/iii.pysdk/packages/python/iii-example/src/main.pysdk/packages/python/iii-example/src/state.pysdk/packages/python/iii-example/src/stream.py
💤 Files with no reviewable changes (1)
- sdk/packages/python/iii-example/src/iii.py
🚧 Files skipped from review as they are similar to previous changes (1)
- sdk/packages/python/iii-example/src/hooks.py
original PR:
iii-hq/sdk#31
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Behavior / Examples
Breaking Changes