-
Notifications
You must be signed in to change notification settings - Fork 509
Consolidate initialization and kwargs passing for AgentOps client #728
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
Consolidate initialization and kwargs passing for AgentOps client #728
Conversation
- Add support for custom exporters via agentops.init(exporter=...) - Add support for custom exporter endpoints via agentops.init(exporter_endpoint=...) - Ensure kwargs are correctly passed downstream to Session and its components - Add tests for custom exporter configuration Co-Authored-By: Constantin-Doru Teodorescu <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
| self.exporter: Optional[Any] = None | ||
| self.exporter_endpoint: Optional[str] = None |
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.
The exporter parameter accepts Any type without validation, which could lead to runtime errors. Should validate the exporter has required methods/interface.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| self.exporter: Optional[Any] = None | |
| self.exporter_endpoint: Optional[str] = None | |
| self.exporter: Optional[MetricsExporter] = None | |
| self.exporter_endpoint: Optional[str] = None |
| elif os.environ.get("AGENTOPS_EXPORTER_ENDPOINT") is not None: | ||
| self.exporter_endpoint = os.environ.get("AGENTOPS_EXPORTER_ENDPOINT") |
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.
Environment variable access is not type-safe - os.environ.get() should specify a default value to ensure consistent type handling.
📝 Committable Code Suggestion
‼️ Ensure you review the code suggestion before committing it to the branch. Make sure it replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| elif os.environ.get("AGENTOPS_EXPORTER_ENDPOINT") is not None: | |
| self.exporter_endpoint = os.environ.get("AGENTOPS_EXPORTER_ENDPOINT") | |
| elif (exporter_endpoint := os.environ.get("AGENTOPS_EXPORTER_ENDPOINT", None)) is not None: | |
| self.exporter_endpoint = exporter_endpoint |
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
Co-Authored-By: Constantin-Doru Teodorescu <[email protected]>
Fixes #721
This PR ensures that kwargs passed to
agentops.init()are correctly passed downstream to Session and its components, including support for custom exporters and exporter endpoints.Key changes:
agentops.init(exporter=...)agentops.init(exporter_endpoint=...)Link to Devin run: https://app.devin.ai/sessions/1e7a27e69cfd4308a9c6a9f67602651e
Requested by: Constantin-Doru