-
Notifications
You must be signed in to change notification settings - Fork 41
v 0.0.7b5 #169
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
v 0.0.7b5 #169
Conversation
- Introduced a new node, DownloadS3FileNode, for downloading files from S3. - Updated main.py to include the new node in the runtime configuration. - Removed unused 'store' field from NodeTemplate model in state-manager. This enhances the cloud storage functionality by allowing file downloads directly from S3.
📝 WalkthroughSummary by CodeRabbit
WalkthroughA new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DownloadS3FileNode
participant AWS_S3
User->>DownloadS3FileNode: Provide bucket_name, key, AWS credentials
DownloadS3FileNode->>AWS_S3: Initiate S3 client with credentials
DownloadS3FileNode->>AWS_S3: Download object (bucket_name, key)
AWS_S3-->>DownloadS3FileNode: Return file data
DownloadS3FileNode-->>User: Return Outputs(file_path)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches
🧪 Generate unit tests
🪧 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 comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Summary of Changes
Hello @NiveditJain, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on enhancing the cloud-storage-runtime by adding a new capability to download files from S3. Concurrently, I've made significant updates to the exospherehost SDK to standardize data handling within nodes, ensuring that all inputs, outputs, and secrets are strictly string-based. This change is accompanied by relevant documentation updates and new validation rules. Additionally, I've performed some general cleanup by removing unused components and simplifying a core data model in the state manager.
Highlights
- New S3 Download Node: I've introduced a new
DownloadS3FileNodewithin thecloud-storage-runtime. This node provides functionality to download specific files from an S3 bucket, requiring the bucket name, file key, and AWS credentials. - Enforced String-Only Data Model for Nodes: I've updated the
exospherehostSDK to enforce a strict string-only data model forInputs,Outputs, andSecretsfields in nodes. This change is reflected in theREADME.mdwith updated examples and new validation logic in the runtime. - Codebase Cleanup and Refactoring: I've removed the
Statusenum and thesample.pyfile from thepython-sdkto streamline the codebase. - NodeTemplate Model Simplification: I've removed the
storefield from theNodeTemplatemodel in thestate-managerto simplify the data structure. - Version Update: I've bumped the
exospherehostSDK version to0.0.7b5.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces version 0.0.7b5, which includes a new DownloadS3FileNode for the cloud storage runtime and enforces a string-only data model for node Inputs, Outputs, and Secrets in the Python SDK. The SDK's README.md has been updated to reflect this significant change. My review focuses on a critical bug in the new S3 download node, a potential issue in a code example in the README, and opportunities for code simplification and robustness. Overall, the changes are a good step towards simplifying the data model, but a few areas need attention to ensure correctness and maintainability.
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
🔭 Outside diff range comments (1)
python-sdk/exospherehost/node/__init__.py (1)
1-11: Docstring still references removedStatus.Lines 8-11 mention “Status constants” but the enum and its re-export were deleted. Remove or update this section to avoid confusing SDK users.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (9)
exosphere-runtimes/cloud-storage-runtime/main.py(2 hunks)exosphere-runtimes/cloud-storage-runtime/nodes/download_s3_file.py(1 hunks)python-sdk/README.md(8 hunks)python-sdk/exospherehost/_version.py(1 hunks)python-sdk/exospherehost/node/__init__.py(1 hunks)python-sdk/exospherehost/node/status.py(0 hunks)python-sdk/exospherehost/runtime.py(1 hunks)python-sdk/sample.py(0 hunks)state-manager/app/models/node_template_model.py(0 hunks)
💤 Files with no reviewable changes (3)
- state-manager/app/models/node_template_model.py
- python-sdk/exospherehost/node/status.py
- python-sdk/sample.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-03T04:26:00.148Z
Learnt from: NiveditJain
PR: exospherehost/exospherehost#131
File: state-manager/app/models/executed_models.py:0-0
Timestamp: 2025-08-03T04:26:00.148Z
Learning: The exospherehost project is currently in beta phase, so breaking changes in APIs are acceptable and don't require versioning or migration strategies.
Applied to files:
python-sdk/exospherehost/_version.py
📚 Learning: 2025-08-02T12:43:35.075Z
Learnt from: NiveditJain
PR: exospherehost/exospherehost#130
File: .github/workflows/ci.yml:22-22
Timestamp: 2025-08-02T12:43:35.075Z
Learning: The exospherehost project requires Python versions > 3.12 for the CI workflow, meaning Python 3.13 or higher should be used despite potential stability concerns with pre-release versions.
Applied to files:
python-sdk/exospherehost/_version.py
🧬 Code Graph Analysis (2)
python-sdk/exospherehost/node/__init__.py (2)
python-sdk/exospherehost/node/status.py (1)
Status(11-44)python-sdk/exospherehost/node/BaseNode.py (1)
BaseNode(6-73)
python-sdk/exospherehost/runtime.py (2)
exosphere-runtimes/cloud-storage-runtime/nodes/list_s3_files.py (2)
Inputs(9-13)Outputs(15-16)python-sdk/exospherehost/node/BaseNode.py (3)
Inputs(29-35)Outputs(37-43)_execute(45-56)
🪛 LanguageTool
python-sdk/README.md
[grammar] ~26-~26: Use the right pronoun
Context: ...Serialize complex data (e.g., JSON) and parse inside your node. ### Basic Node Creat...
(QB_NEW_EN_OTHER_ERROR_IDS_9)
[grammar] ~26-~26: Use correct spacing
Context: ...data (e.g., JSON) and parse inside your node. ### Basic Node Creation Create a simple nod...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (10)
python-sdk/exospherehost/_version.py (1)
1-1: Version bump looks fine.No functional impact beyond metadata.
exosphere-runtimes/cloud-storage-runtime/main.py (1)
4-4: MissingDownloadS3FileNodeimport path check.If
download_s3_file.pysits undernodes/, the relative import is correct. Otherwise the runtime will fail on start-up. Double-check that the file really isexosphere-runtimes/cloud-storage-runtime/nodes/download_s3_file.py.python-sdk/exospherehost/node/__init__.py (1)
15-15: Export set updated—OK.
__all__now only exposesBaseNode, matching the actual public surface.python-sdk/README.md (7)
39-44: Examples correctly reflect v1 string-only modelInputs/Outputs switched to str look consistent with the stated constraint.
47-52: Example execution snippet aligns with string-only constraintThe processing and return shape are clear and correct for v1.
56-59: Runtime init example looks goodNames are strings and node registration is clear.
129-134: Error-handling example stays consistent with string-only outputsThe example conveys failure propagation clearly.
145-145: Import for JSON serialization is appropriateMatches the example usage below.
152-154: Outputs reflect v1 string-only modelLooks consistent and clear.
186-186: Secrets section clearly states string-only constraintConsistent with the rest of the README.
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 (7)
exosphere-runtimes/cloud-storage-runtime/main.py (1)
11-14: Good clarification on node ordering—resolves the earlier nit.Explicitly noting that execution is orchestrated by the state manager addresses the prior comment about implicit ordering.
python-sdk/README.md (6)
26-27: Tighten phrasing in the Important noteConsider changing “parse that string within your node” to “parse it within your node” for concision. Spacing looks good now with the added blank line.
-> Important: In v1, all fields in `Inputs`, `Outputs`, and `Secrets` must be strings. If you need to pass complex data (e.g., JSON), serialize the data to a string first, then parse that string within your node. +> Important: In v1, all fields in `Inputs`, `Outputs`, and `Secrets` must be strings. If you need to pass complex data (e.g., JSON), serialize the data to a string first, then parse it within your node.
76-76: Link to enforcement details or add a short migration noteAdd a pointer to where this is enforced (runtime validation) or a brief migration workflow from non-string models to v1 to reduce confusion.
Would you like me to draft a short “Migration to v1 (string-only models)” subsection and reference the runtime validation check?
109-114: Call out numeric-string expectation for max_lengthSince Inputs are strings, users might pass non-numeric max_length. Add a brief guard/remark to set expectations before converting to int.
Example:
- “max_length must be a numeric string (e.g., '100').”
116-119: Safely parse max_length with a clear errorAvoid confusing runtime errors if max_length is non-numeric.
- max_length = int(self.inputs.max_length) - result = self.inputs.text[:max_length] - return self.Outputs(result=result, length=str(len(result))) + try: + max_length = int(self.inputs.max_length) + except ValueError as e: + raise ValueError("Inputs.max_length must be a numeric string") from e + result = self.inputs.text[:max_length] + return self.Outputs(result=result, length=str(len(result)))
167-171: Raise for HTTP errors to avoid serializing error payloadsCall raise_for_status() so non-2xx responses don’t flow through as “success”.
async with httpx.AsyncClient() as client: http_response = await client.post( f"{self.secrets.api_endpoint}/process", headers=headers, json={"user_id": self.inputs.user_id, "query": self.inputs.query} ) + http_response.raise_for_status()
173-182: Prefer text, then safely fall back to JSON if emptyCurrent logic prioritizes JSON even when text is available and catches broad exceptions. Prefer text first; if empty, try JSON; catch ValueError only.
- # Serialize body: prefer JSON if valid; fallback to text or empty string - response_text = http_response.text or "" - if response_text: - try: - response_str = json.dumps(http_response.json()) - except Exception: - response_str = response_text - else: - response_str = "" + # Serialize non-string data to string (prefer text; fall back to JSON if empty and valid) + response_str = http_response.text + if not response_str: + try: + response_str = json.dumps(http_response.json()) + except ValueError: + response_str = ""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
exosphere-runtimes/cloud-storage-runtime/main.py(1 hunks)python-sdk/README.md(8 hunks)
🧰 Additional context used
🪛 LanguageTool
python-sdk/README.md
[grammar] ~26-~26: Use correct spacing
Context: ...rst, then parse that string within your node. ### Basic Node Creation Create a simple nod...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
🔇 Additional comments (10)
exosphere-runtimes/cloud-storage-runtime/main.py (1)
4-4: Import looks correct and consistent.Importing DownloadS3FileNode mirrors the existing pattern and is aligned with the node registration approach.
python-sdk/README.md (9)
39-39: LGTM: Explicit v1 string-only commentClear and consistent with v1 constraints.
43-43: LGTM: Explicit v1 string-only commentClear and consistent with v1 constraints.
47-52: Example reads well and aligns with v1Good call-out to serialize complex data and return strings.
56-56: LGTM: Namespace shown as a stringMatches the v1 string-only messaging.
129-129: LGTM: Outputs typed as stringsConsistent with v1; example intentionally throws to demonstrate error handling elsewhere.
144-144: LGTM: Import json where usedKeeps the example self-contained.
152-154: LGTM: Output fields as stringsMatches the string-only guidance for v1.
184-186: LGTM: Return stringified payload and statusConsistent with v1 outputs and earlier guidance.
193-193: LGTM: Secrets called out as stringsClear reinforcement of the v1 constraint.
No description provided.