Conversation
Remove automatic loading of STACKONE_ACCOUNT_ID from environment variables in StackOneToolSet initialization to provide more explicit control over account ID configuration. The account_id parameter now requires explicit provision if needed, improving clarity and preventing unexpected behavior from environment variables.
There was a problem hiding this comment.
Pull Request Overview
This PR removes automatic environment variable loading for the STACKONE_ACCOUNT_ID in the StackOneToolSet initialization. The change requires explicit provision of the account_id parameter when needed, improving code clarity and preventing unexpected behavior from environment variables.
Key Changes
- Removed automatic fallback to
os.getenv("STACKONE_ACCOUNT_ID")in StackOneToolSet.init() - Updated docstring and documentation to clarify that account_id requires explicit provision
- Maintained the API key's environment variable fallback for authentication purposes
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| stackone_ai/toolset.py | Removed automatic STACKONE_ACCOUNT_ID environment variable loading and updated docstring |
| CLAUDE.md | Updated documentation to clarify that account_id requires explicit provision |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| toolset = StackOneToolSet( | ||
| api_key="your-api-key", # or STACKONE_API_KEY env var | ||
| account_id="optional-id" # or STACKONE_ACCOUNT_ID env var | ||
| account_id="optional-id" # explicit account ID required |
There was a problem hiding this comment.
The comment 'explicit account ID required' is contradictory since the parameter is still optional (as indicated by 'optional-id'). Consider changing to 'explicit account ID if needed' or similar to clarify that while the parameter is optional, when provided it must be explicit.
| account_id="optional-id" # explicit account ID required | |
| account_id="optional-id" # explicit account ID if needed |
There was a problem hiding this comment.
@ryoppippi good flag here from copilot, is this optional (as the value seems to indicate) or is this required?
There was a problem hiding this comment.
no we can set it later.
| toolset = StackOneToolSet( | ||
| api_key="your-api-key", # or STACKONE_API_KEY env var | ||
| account_id="optional-id" # or STACKONE_ACCOUNT_ID env var | ||
| account_id="optional-id" # explicit account ID required |
There was a problem hiding this comment.
@ryoppippi good flag here from copilot, is this optional (as the value seems to indicate) or is this required?
Summary
This PR removes the automatic loading of the STACKONE_ACCOUNT_ID environment variable from the StackOneToolSet initialization. This change provides more explicit control over account ID configuration.
Changes Made
Rationale
Testing
Test plan
Summary by cubic
Removed automatic loading of STACKONE_ACCOUNT_ID in StackOneToolSet; account_id is now used only when passed explicitly. This makes configuration predictable while keeping STACKONE_API_KEY env fallback; docs updated.