Conversation
* Fix test: Add approve_action for backwards compatibility (no-op) - Added approve_action() method to Antigena class - This method returns dummy success response for backwards compatibility - Modern Darktrace versions replaced approve/decline workflow with direct action methods - Fixes test_antigena_actions test failure * Security: Enable SSL verification by default - Add verify_ssl parameter to DarktraceClient (default: True) - All 28 endpoint modules now use self.client.verify_ssl - Update documentation with SSL verification guidance - Remove urllib3.disable_warnings from examples Closes #47 --------- Co-authored-by: LegendEvent <lpaulmann@example.com>
* feat: add configurable request timeout support - Add timeout parameter to DarktraceClient (default: None for backwards compatibility) - Add per-request timeout override to all endpoint methods - Support tuple format: timeout=(connect_timeout, read_timeout) - Add TimeoutType export for type hints - Add comprehensive test suite (11 tests) - Update README and docs with timeout documentation This enables users to: - Set client-wide timeout: DarktraceClient(timeout=30) - Override per-request: client.advanced_search.search(query, timeout=600) - Use granular timeouts: timeout=(5, 30) for connect/read * fix: use sentinel pattern for timeout to allow None override - Add _UNSET sentinel to distinguish 'not provided' from 'None' - timeout=None now disables timeout (no timeout) - timeout not provided uses client default - Remove unused Union/Tuple imports from client.py - Update tests and documentation Addresses Copilot PR review comments #1 and #2 * feat: add request timing to debug output (closes #50) Add timing information to debug output for all API requests: - New _format_timing() function formats elapsed time as [123ms] or [1.50s] - New _make_request() method in BaseEndpoint logs timing when debug=True - Zero overhead when debug=False (timing only calculated when needed) - All 27 endpoint modules updated to use _make_request() Timing format: DEBUG: GET https://instance.dt/endpoint [123ms] * docs: remove timeout documentation from README, delete test_timeout.py * docs: add BREAKING CHANGE warning for verify_ssl default switch⚠️ BREAKING CHANGE: verify_ssl default changed from False to True in v0.8.56 Users with self-signed certificates must either: 1. Add certificate to system trust store, OR 2. Set verify_ssl=False explicitly - Update README.md with breaking change notice - Update docs/README.md with breaking change warning - Remove timeout documentation (not relevant for most users) * docs: add BREAKING CHANGE warning to all 28 module docs⚠️ BREAKING CHANGE: verify_ssl default changed from False to True in v0.8.56 Updated all module documentation files in docs/modules/ with the breaking change warning for SSL verification default switch.
* feat: add reliability improvements and error handling enhancements - Add connection pooling via requests.Session() for better performance - Add context manager support (__enter__/__exit__/close) for proper resource cleanup - Add automatic retry logic (3 retries, 10s wait) for transient failures (5xx, 429, connection errors) - Add URL scheme validation to block dangerous schemes (file://, ftp://, data://) while allowing private IPs for enterprise deployments - Add _safe_json() helper method for JSON response parsing with error handling - Fix error handling in ModelBreaches to re-raise exceptions instead of returning error dicts - Fix IntelFeed parameter name (fulldetails not full_details) in examples and tests - Update SSL warning suppression to follow SDK's verify_ssl default - Clean up unused imports and translate comments to English * fix: address copilot review comments - Remove unreachable code after raise statements in dt_breaches.py - Fix duplicate imports and ordering in client.py - Remove duplicate SSL warning suppression in conftest.py
## Added - Connection pooling via requests.Session() for 4x faster requests - Context manager support (with DarktraceClient(...) as client) - Automatic retry logic (3 retries, 10s wait for transient failures) - SSRF protection (blocks dangerous URL schemes, allows private IPs) - Configurable request timeout parameter - CHANGELOG.md - tests/test_compilation.py - Full SDK compilation test - tests/test_sdk_readonly.py - Comprehensive read-only test (moved from root) ## Changed - SSL verification now enabled by default (verify_ssl=True) - ModelBreaches methods re-raise exceptions instead of returning error dicts - Fixed IntelFeed fulldetails parameter name in examples - Updated docs/README.md with v0.9.0 features ## Removed - tests/test_devicesearch.py (mocked test replaced by compilation + readonly tests) ## Test Suite - tests/test_compilation.py - 9 tests, no network required - tests/test_sdk_readonly.py - 62 tests against real Darktrace instance
There was a problem hiding this comment.
Pull request overview
This pull request introduces v0.9.0 of the Darktrace SDK, a significant release focused on reliability, performance, and security improvements. The release adds connection pooling with requests.Session(), context manager support for resource cleanup, automatic retry logic for transient failures, SSRF protection through URL scheme validation, and configurable timeouts. A major breaking change is that SSL verification is now enabled by default.
Changes:
- Infrastructure improvements: Connection pooling, context manager support, retry logic (3 retries with 10s wait for 5xx/429/connection errors), SSRF protection, and configurable timeouts
- Security enhancement: SSL verification enabled by default (breaking change)
- Error handling: ModelBreaches methods now re-raise exceptions instead of returning error dicts
- Bug fix: Corrected IntelFeed parameter from
full_detailstofulldetails - Testing: Added compilation test suite (
test_compilation.py) and moved/updated readonly integration tests - Documentation: Updated for v0.9.0 features with SSL verification guidance
Reviewed changes
Copilot reviewed 74 out of 75 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| darktrace/_version.py | Version bump from 0.8.55 to 0.9.0 |
| darktrace/client.py | Added session management, context manager support, SSRF validation, timeout parameter, and verify_ssl default changed to True |
| darktrace/dt_utils.py | Added retry logic via _make_request(), timeout resolution via _resolve_timeout(), timing formatter, and unused _safe_json() helper |
| darktrace/auth.py | Translated German comment to English |
| darktrace/dt_*.py (27 modules) | Added timeout parameter to all methods with proper type hints and _UNSET default (with one exception in dt_antigena.py) |
| darktrace/dt_breaches.py | Changed error handling to re-raise exceptions instead of returning error dicts |
| darktrace/dt_intelfeed.py | Fixed get_with_details() to use correct fulldetails parameter |
| darktrace/dt_antigena.py | Added backwards-compatible no-op approve_action() method |
| tests/test_compilation.py | New comprehensive SDK structure validation test (9 tests, no network required) |
| tests/test_sdk_readonly.py | Updated with correct fulldetails parameter and changed device IDs from 4336 to 1 |
| tests/test_devicesearch.py | Removed (mocked test replaced by compilation + readonly tests) |
| conftest.py | Modified SSL warning suppression logic (but references undefined option) |
| examples/*.py | Updated to use correct fulldetails parameter and removed urllib3 warning suppression |
| docs/README.md | Added v0.9.0 feature documentation and client options table |
| docs/modules/*.md | Added breaking change warning banner to all module docs |
| CHANGELOG.md | New file documenting all v0.9.0 changes |
| README.md | Updated with v0.9.0 features, SSL verification guidance, and certificate installation instructions |
| .gitignore | Added entries for certificates, Python build artifacts, and agent knowledge base files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
darktrace/dt_intelfeed.py
Outdated
| def get_with_details(self): | ||
| """Get intel feed with full details about expiry time and description for each entry.""" | ||
| return self.get(full_details=True) | ||
| return self.get(fulldetails=True) |
There was a problem hiding this comment.
The convenience methods get_sources(), get_by_source(), and get_with_details() don't accept or propagate a timeout parameter, which means users cannot specify custom timeouts when using these methods. Consider adding a timeout parameter to these methods and passing it through to the underlying get() call for consistency. For example: def get_sources(self, timeout=_UNSET): return self.get(sources=True, timeout=timeout)
darktrace/dt_antigena.py
Outdated
|
|
||
| def activate_action( | ||
| self, codeid: int, reason: str = "", duration: Optional[int] = None | ||
| self, codeid: int, reason: str = "", duration: Optional[int] = None, timeout: Optional[Union[float, Tuple[float, float]]] = None |
There was a problem hiding this comment.
The activate_action method uses timeout: Optional[Union[float, Tuple[float, float]]] = None while other methods in this file use timeout: Optional[Union[float, Tuple[float, float]]] = _UNSET. This inconsistency means that for activate_action, explicitly passing timeout=None will result in no timeout (as expected), but for other methods it would fall back to the client's default timeout. For consistency, this should use _UNSET as the default like the other methods.
| self, codeid: int, reason: str = "", duration: Optional[int] = None, timeout: Optional[Union[float, Tuple[float, float]]] = None | |
| self, codeid: int, reason: str = "", duration: Optional[int] = None, timeout: Optional[Union[float, Tuple[float, float]]] = _UNSET # type: ignore[assignment] |
docs/modules/tags.md
Outdated
| @@ -1,5 +1,8 @@ | |||
| # Tags Module | |||
|
|
|||
| > ⚠️ **BREAKING CHANGE**: SSL verification default changed from `False` to `True` in v0.8.56. If using self-signed certificates, you must either add them to your system trust store or set `verify_ssl=False` explicitly. | |||
There was a problem hiding this comment.
The breaking change warning mentions "v0.8.56" but should mention "v0.9.0" since that's when the SSL verification default was actually changed according to the CHANGELOG. This same incorrect version number appears in all module documentation files (tags.md, summarystatistics.md, subnets.md, status.md, similardevices.md, pcaps.md, network.md, models.md, metrics.md, metricdata.md, mbcomments.md, intelfeed.md, filtertypes.md, enums.md, endpointdetails.md, email.md, devicesummary.md, devicesearch.md, devices.md, deviceinfo.md, details.md, cves.md, components.md, breaches.md, auth.md, antigena.md, analyst.md, advanced_search.md).
| def approve_action(self, codeid: int) -> dict: | ||
| """ | ||
| Approve a pending Darktrace RESPOND action (backwards compatibility, no-op). | ||
|
|
||
| This method is retained for backwards compatibility only. In modern Darktrace | ||
| versions, the approve/decline workflow has been replaced by direct action | ||
| management methods. This method is a no-op that returns a success response. | ||
|
|
||
| Args: | ||
| codeid (int): Unique numeric identifier of a RESPOND action (ignored). | ||
|
|
||
| Returns: | ||
| dict: A dummy success response for backwards compatibility. | ||
| """ | ||
| return {"success": True, "message": "Action approved (no-op for backwards compatibility)"} |
There was a problem hiding this comment.
The approve_action() method is documented as a no-op for backwards compatibility but doesn't indicate this is deprecated or provide guidance on what method should be used instead. Consider adding a deprecation warning or at minimum documenting which method should be used instead of this one (likely activate_action()).
darktrace/dt_utils.py
Outdated
| def _safe_json(self, response: requests.Response) -> Any: | ||
| """Parse JSON response with proper error handling. | ||
|
|
||
| Args: | ||
| response: The HTTP response object | ||
|
|
||
| Returns: | ||
| Parsed JSON data (dict or list) | ||
|
|
||
| Raises: | ||
| json.JSONDecodeError: If response body is not valid JSON | ||
| """ | ||
| try: | ||
| return response.json() | ||
| except json.JSONDecodeError as e: | ||
| self.client._debug(f"JSON decode error: {e}") | ||
| raise |
There was a problem hiding this comment.
The _safe_json() helper method is defined in BaseEndpoint but is never used anywhere in the codebase. All endpoint modules continue to call response.json() directly. Either remove this unused method or update the endpoint modules to use it consistently for better error handling.
docs/README.md
Outdated
| | `verify_ssl` | bool | True | Enable SSL certificate verification | | ||
| | `timeout` | int/float | None | Request timeout in seconds (None = no timeout) | | ||
|
|
||
| > ⚠️ **BREAKING CHANGE**: SSL verification default changed from `False` to `True` in v0.8.56. If using self-signed certificates, you must either add them to your system trust store or set `verify_ssl=False` explicitly. |
There was a problem hiding this comment.
The breaking change warning mentions "v0.8.56" but this is a v0.9.0 release. The CHANGELOG indicates the SSL verification default was changed in v0.9.0, not v0.8.56. This inconsistency should be corrected. Either the version number in the warning is wrong, or the CHANGELOG entry is misplaced.
| devices = client.devices.get() | ||
| # Connection automatically closed when exiting block | ||
| ``` | ||
| > ⚠️ **BREAKING CHANGE**: SSL verification default changed from `False` to `True` in v0.8.56. If using self-signed certificates, you must either add them to your system trust store or set `verify_ssl=False` explicitly. |
There was a problem hiding this comment.
Duplicate breaking change warning appears on both line 31 and line 53. The second occurrence at line 53 should be removed as it's redundant.
| # Only suppress SSL warnings when --no-verify is passed | ||
| # Follow SDK's default of verify_ssl=True | ||
| if config.getoption('no_verify', default=False): | ||
| warnings.filterwarnings("ignore", category=InsecureRequestWarning) No newline at end of file |
There was a problem hiding this comment.
The conftest.py change references a --no-verify option that doesn't exist in the pytest_addoption function. The code checks for config.getoption('no_verify', default=False) but this option is never registered. This will cause the code to always use the default value of False, effectively never suppressing SSL warnings. Either add the option registration in pytest_addoption, or remove this conditional logic.
1. dt_intelfeed.py: Add timeout parameter to convenience methods - get_sources(), get_by_source(), get_with_details() now accept timeout 2. dt_antigena.py: Fix timeout default consistency - activate_action() now uses _UNSET instead of None 3. dt_antigena.py: Add deprecation warning to approve_action() - Now emits DeprecationWarning pointing to activate_action() 4. dt_utils.py: Remove unused _safe_json() method 5. docs/README.md: Fix version number and remove duplicate warning - v0.8.56 -> v0.9.0 - Removed duplicate breaking change warning 6. docs/modules/*.md: Fix version number - All module docs now correctly say v0.9.0
The to+hours parameter combination with eventtype may not be supported on all Darktrace versions. Wrap in try/except to handle gracefully.
Release v0.9.0
Added
requests.Session()for 4x faster requestswith DarktraceClient(...) as client:timeoutparameter onDarktraceClienttests/test_compilation.py(9 tests, no network)tests/test_sdk_readonly.py(62 tests against real instance)Changed
verify_ssl=True)fulldetailsparameterRemoved
tests/test_devicesearch.py(mocked test replaced)Test Suite
tests/test_compilation.pypytest tests/test_compilation.py -vtests/test_sdk_readonly.pypytest tests/test_sdk_readonly.py -v --host=... --public-token=... --private-token=...Files Changed
CHANGELOG.md- AddedREADME.md- Updated with v0.9.0 featuresdarktrace/_version.py- 0.8.55 → 0.9.0docs/README.md- Added v0.9.0 features documentationtests/test_compilation.py- Addedtests/test_sdk_readonly.py- Moved from roottests/test_devicesearch.py- Removed