Add HTTP power driver package with client and tests#526
Add HTTP power driver package with client and tests#526mangelajo merged 1 commit intojumpstarter-dev:mainfrom
Conversation
✅ Deploy Preview for jumpstarter-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
WalkthroughA new HTTP-based power control driver, Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant PowerClient
participant HttpPower
participant HTTPServer
User->>PowerClient: call .on() / .off() / .read()
PowerClient->>HttpPower: delegate on/off/read
HttpPower->>HTTPServer: Send HTTP request (on/off/read endpoint)
HTTPServer-->>HttpPower: HTTP response
HttpPower-->>PowerClient: Return result / PowerReading
PowerClient-->>User: Operation complete / reading yielded
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py (1)
22-23: Consider adding a return-type annotation tocycle.All other public methods declare
-> None; annotatingcyclewill improve consistency and static analysis.
Suggest:-def cycle(self, wait: int = 2): +def cycle(self, wait: int = 2) -> None: """Power cycle the device."""packages/jumpstarter-driver-http-power/pyproject.toml (1)
11-16: Consider orderingdependenciesalphabetically.Alphabetical ordering improves readability:
[project] dependencies = [ - "anyio>=4.6.2.post1", - "jumpstarter", - "jumpstarter-driver-power", - "aiohttp", + "aiohttp", + "anyio>=4.6.2.post1", + "jumpstarter", + "jumpstarter-driver-power", ]packages/jumpstarter-driver-http-power/README.md (1)
66-70: Reference the actual driver (HttpPower) instead of the generic client.The API block documents
jumpstarter_driver_power.client.PowerClient, which isn’t the class users of this package will import. Pointing to the concreteHttpPowerclass (or an alias re-exporting the same interface) will prevent confusion and broken Sphinx links.-.. autoclass:: jumpstarter_driver_power.client.PowerClient() +.. autoclass:: jumpstarter_driver_http_power.driver.HttpPower()packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py (2)
58-76: No request timeout → possible indefinite hang.Network calls without a timeout can stall the entire driver. A conservative default (e.g. 10 s) is advisable.
@@ - kwargs = { - 'url': endpoint_config.url, - 'auth': auth, - } + kwargs = { + 'url': endpoint_config.url, + 'auth': auth, + 'timeout': 10, # seconds + }Consider making the timeout configurable in
HttpEndpointConfig.
92-109:read()throws away the HTTP response – consider at least logging it.Even while parsing is TODO, saving the raw response in debug logs (or returning it under a feature flag) would help when integrating real devices.
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py (1)
70-83: Test only covers dummy readings – add an auth path & error handling case.Current assertions verify the happy path with unauthenticated endpoints and dummy values.
Adding:
• a test withauth.basicto hit the newly fixed conversion logic, and
• a test ensuring HTTP errors propagate (e.g. server returns 404)
will hard-enforce the driver’s contract.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
docs/source/reference/package-apis/drivers/index.md(2 hunks)packages/jumpstarter-driver-http-power/README.md(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py(1 hunks)packages/jumpstarter-driver-http-power/pyproject.toml(1 hunks)packages/jumpstarter-driver-power/README.md(1 hunks)packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py(2 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🪛 Pylint (3.3.7)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py
[error] 4-4: Attempted relative import beyond top-level package
(E0402)
[error] 85-85: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 88-88: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 94-94: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 100-100: Instance of 'HTTPServer' has no 'requests' member
(E1101)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: e2e
🔇 Additional comments (6)
pyproject.toml (1)
17-17: Workspace source registration looks good.The new
jumpstarter-driver-http-powerentry is correctly placed alongside other drivers in[tool.uv.sources]and follows the existing grouping convention.packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py (3)
12-13: Good: Added docstring foron.The one-line docstring clearly describes the action.
16-17: Good: Added docstring foroff.The one-line docstring clearly describes the action.
32-35: Good: Expanded docstring forread.The detailed description and
Yieldssection clarify the generator output.docs/source/reference/package-apis/drivers/index.md (2)
26-27: New driver entry is consistent.The
HTTP Poweritem has been added with a clear title and description.
89-90: Includedhttp-power.mdin toctree.The new documentation page is correctly referenced in the hidden toctree.
mangelajo
force-pushed
the
http-power-driver
branch
4 times, most recently
from
e2506f0 to
d9bc67f
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py (2)
24-36: Pre-initialiseserver.requeststo avoid dynamic attribute & silence static-analysis noise
MockHTTPHandlerlazily createsself.server.requests, which leads to:
•pylintE1101 (attribute defined outside__init__)
• Slight race-condition risk if the test accessesserver.requestsbefore the first request is processed.A tiny upfront initialisation keeps linters happy and makes the intent explicit.
- server = HTTPServer(('localhost', 0), MockHTTPHandler) + server = HTTPServer(('localhost', 0), MockHTTPHandler) + # Pre-initialise request log for type checkers & clarity + server.requests: list[dict[str, str | None]] = []Also applies to: 53-57
105-107: Close the listening socket after shutting the server down
HTTPServer.shutdown()stops the serve loop but does not close the underlying socket.
Addingserver.server_close()ensures the port is released immediately and avoids “address already in use” flakiness when tests run in quick succession.- server.shutdown() + server.shutdown() + server.server_close()packages/jumpstarter-driver-http-power/README.md (1)
38-38: Remove trailing punctuation in headingMarkdown-lint (MD026) flags the colon. Dropping it keeps tooling quiet and is stylistically cleaner.
-### Example configuration for Shelly Smart Plug: +### Example configuration for Shelly Smart Plug
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
docs/source/reference/package-apis/drivers/index.md(2 hunks)packages/jumpstarter-driver-http-power/README.md(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py(1 hunks)packages/jumpstarter-driver-http-power/pyproject.toml(1 hunks)packages/jumpstarter-driver-power/README.md(1 hunks)packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py(2 hunks)packages/jumpstarter-driver-yepkit/README.md(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/jumpstarter-driver-yepkit/README.md
- pyproject.toml
🚧 Files skipped from review as they are similar to previous changes (5)
- docs/source/reference/package-apis/drivers/index.md
- packages/jumpstarter-driver-power/README.md
- packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py
- packages/jumpstarter-driver-http-power/pyproject.toml
- packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/jumpstarter-driver-http-power/README.md
38-38: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Pylint (3.3.7)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py
[error] 4-4: Attempted relative import beyond top-level package
(E0402)
[error] 85-85: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 88-88: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 94-94: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 100-100: Instance of 'HTTPServer' has no 'requests' member
(E1101)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (1)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py (1)
75-83: Re-evaluate the expectations forread()resultsThe mock
/readendpoint returns{ "voltage": 12.0, "current": 2.5 }, yet the test asserts0.0.
Either:
- The driver intentionally returns dummy values – in which case documenting this with a TODO would clarify intent; or
- The driver should parse the JSON and surface real readings – which would make the test more meaningful.
Please confirm the intended behaviour and adjust the driver or assertions accordingly.
mangelajo
force-pushed
the
http-power-driver
branch
3 times, most recently
from
68ab870 to
ad5c7e3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/jumpstarter-driver-http-power/README.md (2)
38-38: Fix markdown formatting: Remove trailing punctuation from heading.The heading contains a trailing colon which violates markdown best practices.
-### Example configuration for Shelly Smart Plug: +### Example configuration for Shelly Smart Plug
111-111: Consider enhancing the power reading limitation note.The note about dummy values is helpful for users, but could be more specific about the expected behavior once implemented.
-- The power reading response parsing is not yet implemented. The driver currently returns dummy values (0.0V, 0.0A). +- The power reading response parsing is not yet implemented. The driver currently returns dummy values (0.0V, 0.0A). Future versions will parse JSON responses from the configured power_read endpoint.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
docs/source/reference/package-apis/drivers/http-power.md(1 hunks)docs/source/reference/package-apis/drivers/index.md(2 hunks)packages/jumpstarter-driver-http-power/.gitignore(1 hunks)packages/jumpstarter-driver-http-power/README.md(1 hunks)packages/jumpstarter-driver-http-power/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/__init__.py(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py(1 hunks)packages/jumpstarter-driver-http-power/pyproject.toml(1 hunks)packages/jumpstarter-driver-power/README.md(1 hunks)packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py(2 hunks)packages/jumpstarter-driver-yepkit/README.md(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/init.py
- docs/source/reference/package-apis/drivers/http-power.md
- packages/jumpstarter-driver-http-power/.gitignore
- packages/jumpstarter-driver-http-power/examples/exporter.yaml
🚧 Files skipped from review as they are similar to previous changes (7)
- docs/source/reference/package-apis/drivers/index.md
- packages/jumpstarter-driver-yepkit/README.md
- packages/jumpstarter-driver-power/README.md
- packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py
- pyproject.toml
- packages/jumpstarter-driver-http-power/pyproject.toml
- packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
packages/jumpstarter-driver-http-power/README.md
38-38: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Pylint (3.3.7)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py
[error] 85-85: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 88-88: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 94-94: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 100-100: Instance of 'HTTPServer' has no 'requests' member
(E1101)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: e2e
🔇 Additional comments (5)
packages/jumpstarter-driver-http-power/README.md (1)
92-94: ```shell
#!/bin/bashLocate client.py within the HTTP power driver package
fd 'client.py$' packages/jumpstarter-driver-http-power
Search for PowerClient class definition in the package
grep -R "class PowerClient" -n packages/jumpstarter-driver-http-power
</details> <details> <summary>packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py (4)</summary> `8-49`: **Well-designed mock HTTP server implementation.** The MockHTTPHandler properly simulates HTTP endpoints with request recording and appropriate responses. The implementation handles different HTTP methods and provides realistic JSON responses for the read endpoint. --- `26-27`: **Dynamic attribute assignment is intentional - ignore Pylint warnings.** The code correctly adds a `requests` attribute to the server instance dynamically. The Pylint warnings about "Instance of 'HTTPServer' has no 'requests' member" are false positives since this attribute is created at runtime. The static analysis tool doesn't understand the dynamic nature of this attribute assignment, but the implementation is correct and follows standard Python patterns for extending objects at runtime. --- `51-107`: **Comprehensive test coverage with proper cleanup.** The test function effectively validates: - HTTP request generation for power on/off/read operations - Client method availability and functionality - Request verification (method, path, body) - Proper server lifecycle management The test structure follows best practices with proper setup, execution, verification, and cleanup phases. --- `79-82`: **Test correctly validates current dummy value implementation.** The test appropriately verifies that the driver returns dummy values (0.0V, 0.0A) for power readings, which aligns with the documented limitation in the README. This ensures the test will need to be updated when real response parsing is implemented. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
mangelajo
force-pushed
the
http-power-driver
branch
from
9fce3f0 to
e359ec3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py (2)
43-58: Dictionary conversion logic correctly handles nested structures.The
__post_init__method properly converts dictionary configurations to dataclass instances, including the nestedauth.basicconversion that was flagged in previous reviews. This addresses the runtime AttributeError that would occur when accessing.user/.passwordproperties.
56-57: Previous nested dict conversion issue has been properly addressed.The nested
basicdictionary conversion issue from the previous review has been correctly implemented. This ensures thatself.auth.basicis properly converted from a dict toHttpBasicAuthobject, preventing potentialAttributeErrorwhen accessing.userand.passwordattributes.
🧹 Nitpick comments (8)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py (6)
60-60: Fix formatting: remove extra blank line.Static analysis correctly identifies excessive blank lines before the method definition.
- - def _make_http_request(self, endpoint_config: HttpEndpointConfig) -> str: + def _make_http_request(self, endpoint_config: HttpEndpointConfig) -> str:
94-111: Power reading method needs response parsing implementation.The
readmethod correctly handles the case when no read endpoint is configured and provides appropriate logging. However, the TODO comment indicates that response parsing is not implemented yet.The method currently returns dummy values. Would you like me to help implement response parsing logic to extract voltage and current from JSON responses? For example:
def read(self) -> Generator[PowerReading, None, None]: """Read power measurements via HTTP request""" self.logger.info("Reading power measurements via HTTP") if self.power_read is None: self.logger.error("Power read endpoint not configured") yield PowerReading(voltage=0.0, current=0.0) return response_text = self._make_http_request(self.power_read) try: import json data = json.loads(response_text) voltage = float(data.get('voltage', 0.0)) current = float(data.get('current', 0.0)) yield PowerReading(voltage=voltage, current=current) except (json.JSONDecodeError, ValueError, KeyError) as e: self.logger.error(f"Failed to parse power reading response: {e}") yield PowerReading(voltage=0.0, current=0.0)
59-60: Fix formatting: remove extra blank line.Static analysis detected too many blank lines.
- def _make_http_request(self, endpoint_config: HttpEndpointConfig) -> str:
73-73: Consider security implications of logging URLs.Logging the full URL might expose sensitive information if credentials or tokens are included in the URL parameters.
- self.logger.debug(f"Making {method} request to {endpoint_config.url}") + # Only log the method and host to avoid exposing sensitive URL parameters + from urllib.parse import urlparse + parsed = urlparse(endpoint_config.url) + safe_url = f"{parsed.scheme}://{parsed.netloc}{parsed.path}" + self.logger.debug(f"Making {method} request to {safe_url}")
108-111: Document the incomplete response parsing implementation.The TODO comment indicates that response parsing is not implemented, which aligns with the test expectations. Consider adding more specific documentation about what the expected response format should be.
# TODO: Parse response_text to extract voltage and current values + # Expected response format: {"voltage": <float>, "current": <float>} # For now, return dummy valuesWould you like me to implement the response parsing logic for JSON responses?
75-77: Consider adding more specific error handling for HTTP requests.While
raise_for_status()handles HTTP errors, network-related errors (timeouts, connection errors) could benefit from more specific handling or configuration.response = requests.request(method, **kwargs) response.raise_for_status() + # Consider adding timeout configuration and specific error handling: + # try: + # response = requests.request(method, timeout=self.timeout, **kwargs) + # response.raise_for_status() + # except requests.exceptions.RequestException as e: + # self.logger.error(f"HTTP request failed: {e}") + # raisepackages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py (2)
106-107: Improve thread cleanup robustness.The thread join with timeout doesn't handle the case where the thread fails to join within the timeout period.
finally: server.shutdown() - server_thread.join(timeout=1) + server_thread.join(timeout=1) + if server_thread.is_alive(): + # Log warning or raise exception if thread didn't shut down properly + pass
51-67: Consider adding error handling for server startup.The test assumes the HTTP server will start successfully, but network issues could cause failures.
def test_drivers_http_power(): # Start a mock HTTP server - server = HTTPServer(('localhost', 0), MockHTTPHandler) - server_port = server.server_address[1] - server_thread = threading.Thread(target=server.serve_forever) - server_thread.daemon = True - server_thread.start() + try: + server = HTTPServer(('localhost', 0), MockHTTPHandler) + server_port = server.server_address[1] + server_thread = threading.Thread(target=server.serve_forever) + server_thread.daemon = True + server_thread.start() + except OSError as e: + pytest.skip(f"Could not start test HTTP server: {e}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
docs/source/reference/package-apis/drivers/http-power.md(1 hunks)docs/source/reference/package-apis/drivers/index.md(2 hunks)packages/jumpstarter-driver-http-power/.gitignore(1 hunks)packages/jumpstarter-driver-http-power/README.md(1 hunks)packages/jumpstarter-driver-http-power/examples/exporter.yaml(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/__init__.py(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py(1 hunks)packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py(1 hunks)packages/jumpstarter-driver-http-power/pyproject.toml(1 hunks)packages/jumpstarter-driver-power/README.md(1 hunks)packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py(2 hunks)packages/jumpstarter-driver-yepkit/README.md(1 hunks)pyproject.toml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/jumpstarter-driver-http-power/README.md
🚧 Files skipped from review as they are similar to previous changes (10)
- packages/jumpstarter-driver-power/README.md
- docs/source/reference/package-apis/drivers/http-power.md
- packages/jumpstarter-driver-yepkit/README.md
- packages/jumpstarter-driver-http-power/.gitignore
- docs/source/reference/package-apis/drivers/index.md
- pyproject.toml
- packages/jumpstarter-driver-power/jumpstarter_driver_power/client.py
- packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/init.py
- packages/jumpstarter-driver-http-power/examples/exporter.yaml
- packages/jumpstarter-driver-http-power/pyproject.toml
🧰 Additional context used
🪛 Flake8 (7.2.0)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py
[error] 60-60: too many blank lines (2)
(E303)
🪛 Pylint (3.3.7)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py
[error] 85-85: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 88-88: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 94-94: Instance of 'HTTPServer' has no 'requests' member
(E1101)
[error] 100-100: Instance of 'HTTPServer' has no 'requests' member
(E1101)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.12)
- GitHub Check: pytest-matrix (macos-15, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.11)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.13)
- GitHub Check: pytest-matrix (macos-15, 3.12)
- GitHub Check: pytest-matrix (ubuntu-24.04, 3.11)
- GitHub Check: e2e
🔇 Additional comments (7)
packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver_test.py (4)
8-49: Well-implemented mock HTTP server for testing.The MockHTTPHandler provides comprehensive request recording and appropriate responses for testing. The implementation correctly:
- Suppresses logs during testing
- Records all HTTP methods (GET, POST, PUT)
- Captures request details (method, path, body) for verification
- Provides realistic responses with proper headers and content types
51-107: Comprehensive test coverage with proper setup and teardown.The test function effectively validates:
- HTTP client method availability (
on,off,read)- Actual HTTP request execution and verification
- Request method, path, and body validation
- Proper server lifecycle management with timeout
The test correctly expects dummy values (0.0 voltage/current) which aligns with the current implementation in
driver.py.
85-85: Static analysis false positive - server.requests is dynamically created.The Pylint errors about
server.requestsnot being a member of HTTPServer are false positives. Therequestsattribute is dynamically added to the server instance in theMockHTTPHandler._handle_requestmethod at lines 26-27. This is a common and acceptable pattern for test mocks.Also applies to: 88-88, 94-94, 100-100
24-36: The dynamic attribute assignment is correct, but pylint can't detect it.The pylint warnings about
server.requestsnot being a member are false positives. The code correctly adds therequestsattribute dynamically in line 27, but static analysis tools can't detect this pattern.To suppress these false positives, you could add a pylint disable comment:
self.server.requests.append({ + # pylint: disable=no-member 'method': self.command, 'path': self.path, 'body': body })packages/jumpstarter-driver-http-power/jumpstarter_driver_http_power/driver.py (3)
11-27: Well-structured configuration dataclasses.The configuration classes provide clear separation of concerns:
HttpEndpointConfigfor endpoint details with sensible defaultsHttpBasicAuthfor authentication credentialsHttpAuthConfigfor authentication configurationThe use of
kw_only=Trueenforces clear parameter naming which improves readability.
60-77: Robust HTTP request implementation with proper error handling.The
_make_http_requestmethod correctly:
- Handles authentication when configured
- Supports multiple HTTP methods with appropriate data handling
- Uses
raise_for_status()for proper error handling- Provides debug logging for troubleshooting
79-91: Clean power control methods with appropriate logging.Both
onandoffmethods provide clear logging at info and debug levels, making it easy to track power operations in production environments.
2 participants
Summary by CodeRabbit