Conversation
…ments Co-authored-by: nivedit <nivedit@aikin.club>
Co-authored-by: nivedit <nivedit@aikin.club>
|
Cursor Agent can help with this pull request. Just |
📝 WalkthroughSummary by CodeRabbit
WalkthroughDocumentation updates add a note about Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
README.md(1 hunks)docs/docs/getting-started.md(1 hunks)python-sdk/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
python-sdk/README.md
[grammar] ~62-~62: Use correct spacing
Context: ...) ``` ### Note on blocking behavior of Runtime.start() `Runtime.start()` blocks the main thread when no asyncio ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~64-~64: There might be a problem here.
Context: ...ent loop (e.g., Jupyter), it returns an asyncio.Task and does not block. - Jupyter/interactive ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~64-~64: Use correct spacing
Context: ...ns an asyncio.Task and does not block. - Jupyter/interactive (non-blocking): `...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~66-~66: Use correct spacing
Context: .... - Jupyter/interactive (non-blocking): python runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) task = runtime.start() # background asyncio.Task # await task # optionally wait on it later - Regular sync script (non-blocking via th...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~74-~74: Use correct spacing
Context: ...r sync script (non-blocking via thread): python from threading import Thread runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) Thread(target=runtime.start, daemon=True).start() - From async code (offload to a thread): ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~83-~83: Use correct spacing
Context: ...- From async code (offload to a thread): python import asyncio runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) await asyncio.to_thread(runtime.start) ## Environment Configuration The SDK requi...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
docs/docs/getting-started.md
[grammar] ~89-~89: Use correct spacing
Context: ...) ``` ### Note on blocking behavior of Runtime.start() By design, `Runtime.start()` runs the ru...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~91-~91: There might be a problem here.
Context: ...otebooks), Runtime.start() returns an asyncio.Task and does not block. - If you're in an asyn...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~91-~91: Use correct spacing
Context: ...ns an asyncio.Task and does not block. - If you're in an async/interactive enviro...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~93-~93: Use correct spacing
Context: ...e.g., Jupyter/REPL with a running loop): python # Jupyter/async environment runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) task = runtime.start() # task is an asyncio.Task running in the background # You can continue interacting, and optionally await/cancel the task later # await task # if you want to wait on it - If you need a non-blocking start from a ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~103-~103: Use correct spacing
Context: ...c script, run it in a background thread: python from threading import Thread runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) Thread(target=runtime.start, daemon=True).start() # continue with other work in the main thread - Alternatively, from an async context you...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~113-~113: There might be a mistake here.
Context: ...d - Alternatively, from an async context you can offload to a thread: pyth...
(QB_NEW_EN_OTHER)
[grammar] ~113-~113: Use correct spacing
Context: ...ync context you can offload to a thread: python import asyncio runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) await asyncio.to_thread(runtime.start) ## Next Steps Now that you have the basics...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
README.md
[grammar] ~74-~74: There might be a problem here.
Context: ...ive loop (e.g., Jupyter), it returns an asyncio.Task and does not block. For non-blocking usage ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~74-~74: Use correct spacing
Context: ..., you can run it in a background thread: python from threading import Thread runtime = Runtime(name="my-first-runtime", namespace="hello-world", nodes=[MyFirstNode]) Thread(target=runtime.start, daemon=True).start() - ### Define your first graph Graphs are ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (2)
python-sdk/README.md (1)
62-65: Verified:Runtime.start()correctly callsasyncio.get_running_loop()inside a try‐block, returnsloop.create_task(self._start())when a loop exists, and falls back toasyncio.run(self._start())otherwise, matching the README description.docs/docs/getting-started.md (1)
91-92: Blocking/Task semantics verified
Runtime.start()correctly returns anasyncio.Taskwhen called inside an existing event loop (non-blocking) and blocks the main thread viaasyncio.run(returningNone) otherwise.
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | ||
| await asyncio.to_thread(runtime.start) | ||
| ``` |
There was a problem hiding this comment.
await asyncio.to_thread(...) blocks; schedule it background
This example contradicts “offload to a thread” by awaiting the call. Schedule it as a background task instead.
- await asyncio.to_thread(runtime.start)
+ task = asyncio.create_task(asyncio.to_thread(runtime.start))
+ # optionally await/cancel later📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | |
| await asyncio.to_thread(runtime.start) | |
| ``` | |
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | |
| task = asyncio.create_task(asyncio.to_thread(runtime.start)) | |
| # optionally await or cancel the task later |
🤖 Prompt for AI Agents
In docs/docs/getting-started.md around lines 118 to 120, the example awaits
asyncio.to_thread(runtime.start) which blocks the coroutine instead of
offloading startup to the background; change it to schedule the call as a
background task (e.g., wrap the to_thread call in a task creation so
runtime.start runs on a thread without awaiting it) so the example demonstrates
non-blocking background scheduling.
| - Regular sync script (non-blocking via thread): | ||
|
|
||
| ```python | ||
| from threading import Thread | ||
|
|
||
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | ||
| Thread(target=runtime.start, daemon=True).start() | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Keep a thread handle; call out daemon trade-offs
Create and name the thread so users can observe/coordinate shutdown. Daemon threads can terminate abruptly; a handle enables optional joining.
- Thread(target=runtime.start, daemon=True).start()
+ t = Thread(target=runtime.start, name="exosphere-runtime", daemon=True)
+ t.start()
+ # on shutdown you may: t.join(timeout=5)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Regular sync script (non-blocking via thread): | |
| ```python | |
| from threading import Thread | |
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | |
| Thread(target=runtime.start, daemon=True).start() | |
| ``` | |
| from threading import Thread | |
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | |
| t = Thread(target=runtime.start, name="exosphere-runtime", daemon=True) | |
| t.start() | |
| # on shutdown you may: t.join(timeout=5) |
🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: Use correct spacing
Context: ...r sync script (non-blocking via thread): python from threading import Thread runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) Thread(target=runtime.start, daemon=True).start() - From async code (offload to a thread): ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In python-sdk/README.md around lines 74 to 81, the example spawns a daemon
thread without keeping a handle or naming it; update the guidance to create and
assign the Thread to a variable, give it a descriptive name, and avoid using
daemon=True by default (or clearly call out the abrupt-termination trade-off) so
callers can optionally call thread.join() during shutdown to coordinate clean
termination; mention that if daemon is used it should be an explicit choice with
warning about abrupt exit.
| ```python | ||
| import asyncio | ||
|
|
||
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | ||
| await asyncio.to_thread(runtime.start) | ||
| ``` |
There was a problem hiding this comment.
Awaiting asyncio.to_thread will block; schedule it instead
await asyncio.to_thread(runtime.start) blocks the current coroutine indefinitely if start() runs forever. Schedule it as a background task.
- await asyncio.to_thread(runtime.start)
+ task = asyncio.create_task(asyncio.to_thread(runtime.start))
+ # optionally: await task # later, if you want to join/cancelAlternative: asyncio.get_running_loop().run_in_executor(None, runtime.start) (also schedule without awaiting).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ```python | |
| import asyncio | |
| runtime = Runtime(namespace="MyProject", name="DataProcessor", nodes=[SampleNode]) | |
| await asyncio.to_thread(runtime.start) | |
| ``` |
🤖 Prompt for AI Agents
In python-sdk/README.md around lines 85 to 90, the example uses "await
asyncio.to_thread(runtime.start)" which will block the current coroutine if
runtime.start() runs indefinitely; instead schedule it as a background task or
run it in an executor without awaiting. Replace the await with creating a
background task (e.g., asyncio.create_task(asyncio.to_thread(runtime.start))) or
use the running loop's run_in_executor and do not await its result, ensuring
runtime.start runs concurrently and does not block the calling coroutine.
| Note: `Runtime.start()` will block the main thread in normal scripts (no running event loop). In interactive environments with an active loop (e.g., Jupyter), it returns an `asyncio.Task` and does not block. For non-blocking usage from a sync script, you can run it in a background thread: | ||
|
|
||
| ```python | ||
| from threading import Thread | ||
|
|
||
| runtime = Runtime(name="my-first-runtime", namespace="hello-world", nodes=[MyFirstNode]) | ||
| Thread(target=runtime.start, daemon=True).start() | ||
| ``` |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Background-thread sample: capture the thread handle and note shutdown behavior
Starting a daemon thread is fine for quickstarts, but users should keep a handle for observability/shutdown. Replace the one-liner with two lines.
- Thread(target=runtime.start, daemon=True).start()
+ t = Thread(target=runtime.start, name="exosphere-runtime", daemon=True)
+ t.start() # optionally join on shutdown: t.join(timeout=5)If Runtime.start() truly returns an asyncio.Task when a loop is running, please verify in code (e.g., look for loop-detection and asyncio.create_task paths) so the note remains accurate across releases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Note: `Runtime.start()` will block the main thread in normal scripts (no running event loop). In interactive environments with an active loop (e.g., Jupyter), it returns an `asyncio.Task` and does not block. For non-blocking usage from a sync script, you can run it in a background thread: | |
| ```python | |
| from threading import Thread | |
| runtime = Runtime(name="my-first-runtime", namespace="hello-world", nodes=[MyFirstNode]) | |
| Thread(target=runtime.start, daemon=True).start() | |
| ``` | |
| from threading import Thread | |
| runtime = Runtime(name="my-first-runtime", namespace="hello-world", nodes=[MyFirstNode]) | |
| t = Thread(target=runtime.start, name="exosphere-runtime", daemon=True) | |
| t.start() # optionally join on shutdown: t.join(timeout=5) |
🧰 Tools
🪛 LanguageTool
[grammar] ~74-~74: There might be a problem here.
Context: ...ive loop (e.g., Jupyter), it returns an asyncio.Task and does not block. For non-blocking usage ...
(QB_NEW_EN_MERGED_MATCH)
[grammar] ~74-~74: Use correct spacing
Context: ..., you can run it in a background thread: python from threading import Thread runtime = Runtime(name="my-first-runtime", namespace="hello-world", nodes=[MyFirstNode]) Thread(target=runtime.start, daemon=True).start() - ### Define your first graph Graphs are ...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🤖 Prompt for AI Agents
In README.md around lines 74 to 81, replace the one-liner daemon-thread example
with a two-line pattern that stores the Thread object for observability and
graceful shutdown (i.e., create the Thread, assign it to a variable, then start
it), and add a brief note about shutdown behavior and why keeping the handle
matters; additionally, verify the assertion that Runtime.start() returns an
asyncio.Task when an event loop is running by inspecting the runtime
implementation for loop-detection logic and any paths that call
asyncio.create_task (or similar), and if that behavior differs, update the
README note to reflect the actual behavior.
Add documentation for
Runtime.start()blocking behavior and provide non-blocking examples, fixing #280.Slack Thread