Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for an alternative Happysocks Socket.IO namespace (/v1/engine-ch) intended for ClickHouse-backed storage, making it selectable via a new BlazeMeter module setting and plumbed through HappysocksClient.
Changes:
- Add ClickHouse namespace support to
HappysocksEngineNamespace/HappysocksClientand use it for connect/emit. - Expose a
happysocks-use-clickhousesetting in the BlazeMeter reporter to enable the new namespace. - Add/extend unit tests and add a docs changelog entry.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
bzt/utils.py |
Introduces /v1/engine-ch namespace selection and uses selected namespace for connect/emit. |
bzt/modules/blazemeter/blazemeter_reporter.py |
Adds happysocks-use-clickhouse setting and passes it to HappysocksClient. |
tests/unit/test_bza.py |
Adds coverage for ClickHouse namespace behavior and namespace selection tests. |
tests/unit/modules/test_blazeMeterUploader.py |
Adds a test verifying reporter passes use_clickhouse to HappysocksClient. |
site/dat/docs/changes/feat-support-for-clickhouse-happysocks-namespace.change |
Records the feature addition in the changelog system. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| use_clickhouse = self.settings.get("happysocks-use-clickhouse", False) | ||
| self.happysocks_client = HappysocksClient(happysocks_address, self._sess_id, signature, | ||
| happysocks_verbose_logging, happysocks_verify_ssl, | ||
| request_timeout, connect_timeout) | ||
| request_timeout, connect_timeout, use_clickhouse) |
There was a problem hiding this comment.
HappysocksClient is instantiated with a long list of positional arguments, which is easy to mis-order now that use_clickhouse has been added. Consider switching the optional params (e.g. request/connect timeouts and use_clickhouse) to keyword arguments to make the call self-documenting and more robust to future signature changes.
| args, _ = mock_client_class.call_args | ||
| # args expected: address, session_id, signature, verbose_logging, verify_ssl, request_timeout, connect_timeout, use_clickhouse | ||
| self.assertFalse(args[7]) # default use_clickhouse | ||
|
|
||
| # 2. use_clickhouse = True | ||
| reporter = BlazeMeterUploader() | ||
| reporter.engine = EngineEmul() | ||
| reporter.parameters['signature'] = '123' | ||
| reporter.parameters['session-id'] = 'sess1' | ||
| reporter.parameters['master-id'] = 122362 | ||
| reporter.settings['happysocks-address'] = 'https://unknown/hs' | ||
| reporter.settings['happysocks-use-clickhouse'] = True | ||
| reporter.prepare() | ||
|
|
||
| args, _ = mock_client_class.call_args | ||
| self.assertTrue(args[7]) # explicit use_clickhouse=True |
There was a problem hiding this comment.
This test relies on args[7] positional indexing to verify use_clickhouse. That’s brittle if the HappysocksClient constructor ever changes or if the production code switches to keyword args. Prefer asserting via mock_client_class.call_args.kwargs.get('use_clickhouse') (or otherwise matching by parameter name) to keep the test resilient.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1973 +/- ##
==========================================
- Coverage 89.91% 89.85% -0.06%
==========================================
Files 71 71
Lines 20182 20183 +1
==========================================
- Hits 18145 18133 -12
- Misses 2037 2050 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Each PR must conform to Developer's Guide.
Quick checklist:
site/dat/docs/changesdirectory, one-line note of change inside