-
Notifications
You must be signed in to change notification settings - Fork 41
Adding Secrets functionality #149
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
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughSummary by CodeRabbit
WalkthroughThis update introduces encrypted secret management to graph templates. It adds secret fields to request and response models, implements AES-GCM encryption/decryption utilities, and extends the graph template model with secret storage, validation, and accessor methods. The upsert controller is updated to handle secrets securely during graph template creation and update. Additionally, node registration now supports a secrets attribute with validation and response updates, and the SDK runtime enforces secret schema correctness. New API endpoints enable retrieval of secrets for states, and states now track the associated graph name. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Controller
participant GraphTemplateModel
participant Encrypter
Client->>Controller: UpsertGraphTemplateRequest (with secrets)
Controller->>GraphTemplateModel: set_secrets(body.secrets)
GraphTemplateModel->>Encrypter: encrypt(secret_value) for each secret
Encrypter-->>GraphTemplateModel: encrypted_secret
Controller->>GraphTemplateModel: insert() or update()
Controller->>GraphTemplateModel: get_secrets()
GraphTemplateModel->>Encrypter: decrypt(encrypted_secret) for each secret
Encrypter-->>GraphTemplateModel: secret_value
Controller-->>Client: UpsertGraphTemplateResponse (secrets presence map)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✨ 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 (
|
|
Fixing secrets as in this issue: #140 |
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
state-manager/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
state-manager/app/controller/upsert_graph_template.py(2 hunks)state-manager/app/models/db/graph_template_model.py(2 hunks)state-manager/app/models/graph_models.py(1 hunks)state-manager/app/utils/encrypter.py(1 hunks)state-manager/pyproject.toml(1 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
📚 Learning: the exospherehost project requires python versions > 3.12 for the ci workflow, meaning python 3.13 o...
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:
state-manager/pyproject.toml
📚 Learning: in the exospherehost codebase, for upsert operations on graph templates, the team prioritizes api id...
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
Applied to files:
state-manager/app/controller/upsert_graph_template.py
🔇 Additional comments (12)
state-manager/pyproject.toml (1)
9-9: cryptography>=45.0.5 verified on PyPI and secured
- Version 45.0.5 is published on PyPI and is the current latest release.
- All known security advisories (including the OpenSSL issue patched in 44.0.1) apply only to versions < 44.0.1.
- Since 45.0.5 > 44.0.1, it includes all fixes and has no outstanding vulnerabilities in its version range.
No further changes required here.
state-manager/app/utils/encrypter.py (4)
7-9: LGTM!The key generation method correctly creates a 256-bit AES key with proper URL-safe base64 encoding.
11-21: Well-implemented key initialization with proper validation.The constructor correctly validates the encryption key from the environment variable, ensuring it's properly formatted base64 and exactly 32 bytes for AES-256.
23-26: Secure encryption implementation.The method correctly uses a 12-byte random nonce, prepends it to the ciphertext, and returns URL-safe base64 encoded output.
28-32: Correct decryption implementation.The method properly extracts the nonce and ciphertext, then decrypts and decodes the result.
state-manager/app/models/graph_models.py (1)
9-9: Good security design for secrets handling.The API design correctly accepts plaintext secrets in requests and returns only presence indicators in responses, preventing secret value exposure.
Also applies to: 15-15
state-manager/app/controller/upsert_graph_template.py (3)
21-27: Clean implementation of secret handling during update.The chaining of
set_secrets()withupdate()ensures secrets are properly encrypted before database operations.
36-44: Consistent secret handling for new templates.The implementation properly encrypts secrets before insertion, maintaining consistency with the update operation.
50-50: Secure response construction.The response correctly exposes only the presence of secrets without revealing their values.
state-manager/app/models/db/graph_template_model.py (3)
1-2: Proper setup for secrets functionality.The imports and field definition are appropriate, with correct use of
default_factoryto avoid mutable default issues.Also applies to: 5-5, 10-11, 19-19
30-42: Comprehensive validation of secrets dictionary.The validator properly checks for empty values and correct types before delegating to value-specific validation.
70-72: Good use of fluent interface pattern.Returning
selffromset_secretsenables clean method chaining as demonstrated in the controller.
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: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
python-sdk/exospherehost/node/BaseNode.py(1 hunks)python-sdk/exospherehost/runtime.py(2 hunks)state-manager/app/controller/register_nodes.py(3 hunks)state-manager/app/models/db/registered_node.py(1 hunks)state-manager/app/models/register_nodes_request.py(1 hunks)state-manager/app/models/register_nodes_response.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
🪛 GitHub Actions: Ruff check on changed files only
python-sdk/exospherehost/runtime.py
[error] 261-261: Ruff: Use is and is not for type comparisons, or isinstance() for isinstance checks (E721) at line 261.
🔇 Additional comments (10)
state-manager/app/models/register_nodes_response.py (1)
9-9: LGTM!The addition of the
secretsfield follows the established pattern and aligns with the broader secret management functionality being introduced across the system.state-manager/app/models/register_nodes_request.py (2)
9-9: LGTM!The
secretsfield addition is consistent with the corresponding response model and follows proper Pydantic field definition patterns.
14-14: Good housekeeping.Removing trailing whitespace improves code cleanliness.
state-manager/app/models/db/registered_node.py (1)
12-12: Good housekeeping.Removing trailing whitespace improves code cleanliness.
python-sdk/exospherehost/node/BaseNode.py (1)
45-49: LGTM!The
Secretsclass follows the established pattern ofInputsandOutputsnested classes, providing a clean foundation for nodes to define their secret schemas.python-sdk/exospherehost/runtime.py (2)
138-140: LGTM!The secrets field registration correctly extracts field names from the node's Secrets model, maintaining consistency with the inputs and outputs schema registration.
255-259: Excellent validation logic for Secrets schema.The validation ensures that nodes properly define their Secrets class and inherit from BaseModel, maintaining consistency with the existing Inputs and Outputs validation pattern.
state-manager/app/controller/register_nodes.py (3)
46-47: LGTM! Consistent secrets handling in node creation.The addition of the secrets parameter to the RegisteredNode constructor maintains consistency with the update operation and ensures proper initialization of the secrets field for new nodes.
56-57: LGTM! Complete end-to-end secrets handling.The inclusion of secrets in the response model completes the end-to-end implementation, ensuring that secrets are properly handled from request input through database storage to API response.
33-34: Secrets field integration looks solid—just clean up the type‐checker noiseI’ve confirmed that:
- The DB model (
RegisteredNode), request model (NodeRegistrationModel), and response model (RegisteredNodeModel) all declaresecrets: list[str]/List[str]consistently.- The addition of
RegisteredNode.secrets: node_data.secretsin your update, creation, and response mappings aligns with inputs_schema/outputs_schema.The only remaining item is the
# type: ignoreon lines 33–34 (and similarly at lines 46–47, 56–57) when you mapRegisteredNode.secrets. That’s silencing a typing mismatch between SQLAlchemy’s column attribute and your data shape. You can:
- Verify that
secretsis declared as a SQLAlchemy column (e.g.Column(ARRAY(String))) inRegisteredNode—this often satisfies most type checkers.- If the warning persists, narrow the ignore (e.g.
# type: ignore[assignment]) or add an explicit cast instead of a blanket# type: ignore.No functional changes needed beyond cleaning up those mypy/flake8 warnings.
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: 2
♻️ Duplicate comments (1)
python-sdk/exospherehost/runtime.py (1)
277-284: Fix type comparison per pipeline failure.The validation logic for secrets is correct, but line 283 still uses
!=instead ofis notfor type comparison, which causes pipeline failure as noted in the previous review.for field_name, field_info in node.Secrets.model_fields.items(): - if field_info.annotation != str: + if field_info.annotation is not str: errors.append(f"{node.__name__}.Secrets field '{field_name}' must be of type str, got {field_info.annotation}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
python-sdk/exospherehost/node/BaseNode.py(2 hunks)python-sdk/exospherehost/runtime.py(6 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
🔇 Additional comments (6)
python-sdk/exospherehost/node/BaseNode.py (2)
45-49: LGTM! Consistent with existing patterns.The
Secretsclass follows the same design pattern as the existingInputsandOutputsclasses, maintaining consistency in the codebase architecture.
62-62: LGTM! Consistent assignment pattern.The secrets assignment follows the same pattern as inputs assignment, making secrets accessible to node implementations via
self.secrets.python-sdk/exospherehost/runtime.py (4)
4-4: LGTM! Required import for new method.The
Dictimport is needed for the return type annotation of the new_get_secretsmethod.
120-124: LGTM! Consistent endpoint construction pattern.The method follows the same pattern as other endpoint construction methods and correctly includes the state_id in the URL path.
144-146: LGTM! Correct secrets registration.The implementation correctly extracts secret field names from the node's
Secretsmodel and includes them in the registration payload, aligning with the broader secrets functionality.
307-308: LGTM! Correct secrets integration in worker.The implementation correctly fetches secrets for each state and passes them to the node's
_executemethod alongside inputs, completing the integration with the updatedBaseNodeinterface.
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 (6)
state-manager/app/utils/encrypter.py (1)
34-49: LGTM! Lazy initialization properly implemented.The singleton pattern with lazy initialization has been correctly implemented as suggested in the previous review. This prevents application failures when the encryption key is not needed.
state-manager/app/models/db/graph_template_model.py (2)
44-63: LGTM! Base64 validation correctly implemented.The URL-safe base64 validation has been properly implemented without the problematic divisibility check. The validation correctly relies on base64.urlsafe_b64decode to handle padding automatically.
74-79: LGTM! Error handling properly implemented.The method now correctly returns
Nonewhen the secret is not found, addressing the previous concern about KeyError exceptions.python-sdk/exospherehost/node/BaseNode.py (1)
51-61: LGTM! Docstring properly updated.The docstring has been correctly updated to document the
secretsparameter, addressing the previous review comment.python-sdk/exospherehost/runtime.py (2)
283-285: LGTM! Type comparison correctly implemented.The type comparison now correctly uses
is notinstead of!=, addressing the pipeline failure from the previous review.
235-250: LGTM! Error handling properly implemented.The method now correctly returns an empty dictionary when the request fails, preventing downstream issues when constructing the Secrets object.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
python-sdk/exospherehost/node/BaseNode.py(1 hunks)python-sdk/exospherehost/runtime.py(6 hunks)state-manager/app/models/db/graph_template_model.py(2 hunks)state-manager/app/utils/encrypter.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
🔇 Additional comments (1)
python-sdk/exospherehost/runtime.py (1)
308-309: Consider handling missing required secrets.When
_get_secretsreturns an empty dictionary due to an error, thenode.Secrets(**secrets)instantiation might fail if the node's Secrets class has required fields.Consider adding error handling to provide a more informative error message:
- secrets = await self._get_secrets(state["state_id"]) - outputs = await node()._execute(node.Inputs(**state["inputs"]), node.Secrets(**secrets)) + secrets = await self._get_secrets(state["state_id"]) + try: + secrets_obj = node.Secrets(**secrets) + except ValidationError as e: + if not secrets: # Empty dict from error + raise RuntimeError(f"Failed to fetch secrets for node {node.__name__}: {e}") + raise + outputs = await node()._execute(node.Inputs(**state["inputs"]), secrets_obj)
- Introduced a new section in the README to explain how to work with secrets, including a code example for using secrets in nodes. - Enhanced the BaseNode class to define a schema for secrets, ensuring secure handling and validation. - Added a new route to retrieve secrets for a specific state, along with the necessary controller and response model. - Updated state management models to include graph names and ensure compatibility with the new secrets functionality.
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
.github/workflows/deploy-kubernetes.yml(1 hunks)python-sdk/README.md(1 hunks)python-sdk/exospherehost/_version.py(1 hunks)python-sdk/exospherehost/node/BaseNode.py(1 hunks)python-sdk/exospherehost/runtime.py(6 hunks)state-manager/app/controller/create_states.py(2 hunks)state-manager/app/controller/get_secrets.py(1 hunks)state-manager/app/models/create_models.py(1 hunks)state-manager/app/models/db/state.py(1 hunks)state-manager/app/models/secrets_response.py(1 hunks)state-manager/app/routes.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: NiveditJain
PR: exospherehost/exospherehost#132
File: state-manager/app/controller/upsert_graph_template.py:11-27
Timestamp: 2025-08-03T16:46:04.030Z
Learning: In the exospherehost codebase, for upsert operations on graph templates, the team prioritizes API idempotency over avoiding race conditions in the database layer implementation. The approach of separate find and insert/update operations is acceptable when the API behavior remains consistent.
📚 Learning: the exospherehost project is currently in beta phase, so breaking changes in apis are acceptable and...
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: the exospherehost project requires python versions > 3.12 for the ci workflow, meaning python 3.13 o...
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
🪛 LanguageTool
python-sdk/README.md
[grammar] ~133-~133: Use correct spacing
Context: ...nd the state manager. ### Working with Secrets Secrets allow you to securely manage sen...
(QB_NEW_EN_OTHER_ERROR_IDS_5)
[grammar] ~135-~135: There might be a mistake here.
Context: ...kens. Here's how to use secrets in your nodes: python from exospherehost import Runtime, BaseNode from pydantic import BaseModel class APINode(BaseNode): class Inputs(BaseModel): user_id: str query: str class Outputs(BaseModel): response: dict status: str class Secrets(BaseModel): api_key: str api_endpoint: str database_url: str async def execute(self) -> Outputs: # Access secrets via self.secrets headers = {"Authorization": f"Bearer {self.secrets.api_key}"} # Use secrets for API calls import httpx async with httpx.AsyncClient() as client: response = await client.post( f"{self.secrets.api_endpoint}/process", headers=headers, json={"user_id": self.inputs.user_id, "query": self.inputs.query} ) return self.Outputs( response=response.json(), status="success" ) **Ke...
(QB_NEW_EN_OTHER)
[grammar] ~174-~174: There might be a mistake here.
Context: ...cess" ) ``` Key points about secrets: - Security: Secrets are stored securely...
(QB_NEW_EN_OTHER)
[grammar] ~176-~176: There might be a mistake here.
Context: ... and are never exposed in logs or error messages - Validation: The Secrets class uses P...
(QB_NEW_EN_OTHER)
[grammar] ~177-~177: There might be a mistake here.
Context: ...ntic for automatic validation of secret values - Access: Secrets are available via `sel...
(QB_NEW_EN_OTHER)
[grammar] ~178-~178: There might be a mistake here.
Context: ...vailable via self.secrets during node execution - Types: Common secret types include API...
(QB_NEW_EN_OTHER)
[grammar] ~179-~179: There might be a mistake here.
Context: ...ls, encryption keys, and authentication tokens - Injection: Secrets are injected by the...
(QB_NEW_EN_OTHER)
[grammar] ~180-~180: There might be a mistake here.
Context: ... time, so you don't need to handle them manually ## Integration with ExosphereHost Platform ...
(QB_NEW_EN_OTHER)
🪛 GitHub Actions: Ruff check on changed files only
state-manager/app/routes.py
[error] 30-30: Ruff F401: .models.db.state.State imported but unused. Remove unused import.
[error] 168-168: Ruff F541: f-string without any placeholders. Remove extraneous f prefix.
[error] 170-170: Ruff F541: f-string without any placeholders. Remove extraneous f prefix.
state-manager/app/controller/get_secrets.py
[error] 4-4: Ruff F401: app.models.db.registered_node.RegisteredNode imported but unused. Remove unused import.
🔇 Additional comments (19)
.github/workflows/deploy-kubernetes.yml (1)
34-34: LGTM! Proper secrets management setup.The addition of
SECRETS_ENCRYPTION_KEYto the Kubernetes secret follows the established pattern and correctly supports the new AES-GCM encryption functionality for the state manager.python-sdk/exospherehost/_version.py (1)
1-1: Appropriate version bump for new functionality.The version increment from "0.0.7b2" to "0.0.7b3" correctly reflects the addition of secrets management features in this beta release.
state-manager/app/models/db/state.py (1)
11-11: LGTM! Essential field for secrets functionality.The addition of the
graph_namefield properly links states to their graph templates, enabling secrets retrieval as described in the PR objectives. The field follows the established pattern with proper typing and documentation.state-manager/app/models/create_models.py (2)
9-9: Consistent model extension for graph template association.The addition of
graph_nametoRequestStateModelproperly enables clients to specify the graph template when creating states, maintaining consistency with the database model changes.
16-16: Proper response model alignment.The addition of
graph_nametoResponseStateModelensures API responses include graph template information, maintaining consistency across request, response, and database models.state-manager/app/models/secrets_response.py (1)
1-6: Well-designed secrets response model.The
SecretsResponseModelis properly implemented with:
- Correct typing for secret name-to-value mapping
- Appropriate Pydantic configuration
- Clear documentation
- Simple, focused design for the secrets API endpoint
state-manager/app/controller/create_states.py (1)
21-21: LGTM! Properly integrates graph_name field.The addition of
graph_namefield is correctly implemented in both state creation and response model, maintaining consistency with the data flow.Also applies to: 39-39
state-manager/app/routes.py (1)
157-174: Well-structured secrets retrieval endpoint.The endpoint implementation follows the established patterns and properly handles authentication, request tracking, and delegation to the controller.
python-sdk/README.md (1)
133-181: Excellent documentation for the secrets feature!The new section provides clear explanations, practical examples, and important security considerations. The code example effectively demonstrates how to define and use secrets within nodes.
python-sdk/exospherehost/runtime.py (3)
284-284: Type comparison correctly implemented.Good to see the previous review comment has been addressed - using
is notfor type comparison is the correct approach.
235-250: Proper error handling for secrets retrieval.The method correctly returns an empty dictionary on failure, preventing downstream issues. The error logging provides good visibility into failures.
120-125: Comprehensive secrets integration in the runtime.The implementation properly handles all aspects of secret management:
- Registration of secret schemas with the state manager
- Validation that nodes define proper Secrets classes with string fields
- Secure retrieval of secrets per state
- Clean injection into node execution
This provides a secure and developer-friendly way to manage sensitive configuration.
Also applies to: 144-146, 278-285, 308-309
python-sdk/exospherehost/node/BaseNode.py (3)
72-76: Docstring properly updated for the secrets parameter.The previous review comment has been addressed - the docstring now correctly documents both the
inputsandsecretsparameters.
45-68: Excellent documentation for the Secrets class!The expanded docstring provides comprehensive information about:
- Purpose and usage of secrets
- Types of data that should be stored as secrets
- Security considerations
- Examples of common secret types
This addresses the previous review comment perfectly and will greatly help developers understand how to use this feature.
70-83: Clean integration of secrets into node execution.The implementation elegantly extends the node execution model to support secrets while maintaining the existing API. Storing secrets in
self.secretsfollows the established pattern withself.inputs.state-manager/app/controller/get_secrets.py (4)
11-25: Well-structured function signature and documentation.The function signature is clear with proper type hints, and the docstring follows good documentation practices with comprehensive Args, Returns, and Raises sections.
28-36: Proper state validation with security check.The implementation correctly validates state existence and performs an important security check to ensure the state belongs to the requested namespace, preventing unauthorized access to secrets from other namespaces.
39-46: Appropriate graph template lookup logic.The graph template lookup correctly uses both the graph name from the state and the namespace for proper isolation. The error handling with logging provides good debugging information.
49-53: Clean secret retrieval and response construction.The implementation delegates secret decryption to the
GraphTemplate.get_secrets()method, which is a good separation of concerns. The logging provides useful information about the number of secrets retrieved.
| except Exception as e: | ||
| logger.error(f"Error getting secrets for state {state_id}: {str(e)}", x_exosphere_request_id=x_exosphere_request_id) | ||
| raise e No newline at end of 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.
🧹 Nitpick (assertive)
Consider improving exception re-raising pattern.
While the exception handling logs errors properly, using bare raise e is less idiomatic than raise alone, as it can potentially modify the stack trace.
- raise e
+ raise🤖 Prompt for AI Agents
In state-manager/app/controller/get_secrets.py around lines 55 to 57, the
exception is re-raised using 'raise e', which can alter the original stack
trace. Replace 'raise e' with a bare 'raise' to re-raise the caught exception
while preserving the original traceback.
No description provided.