-
Notifications
You must be signed in to change notification settings - Fork 22
Simplify SNMP driver #803
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
base: main
Are you sure you want to change the base?
Simplify SNMP driver #803
Changes from all commits
eef120a
7f459c7
1f48f94
8d8a497
78caab3
c2b88b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,9 +1,11 @@ | ||||||||||||||||
| import asyncio | ||||||||||||||||
| import socket | ||||||||||||||||
| from collections.abc import AsyncGenerator | ||||||||||||||||
| from dataclasses import dataclass, field | ||||||||||||||||
| from enum import Enum, IntEnum | ||||||||||||||||
| from typing import Any, Dict, Tuple | ||||||||||||||||
| from typing import Any, Dict | ||||||||||||||||
|
|
||||||||||||||||
| from anyio import Event, fail_after | ||||||||||||||||
| from jumpstarter_driver_power.driver import PowerInterface, PowerReading | ||||||||||||||||
| from pysnmp.carrier.asyncio.dgram import udp | ||||||||||||||||
| from pysnmp.entity import config, engine | ||||||||||||||||
| from pysnmp.entity.rfc3413 import cmdgen | ||||||||||||||||
|
|
@@ -36,7 +38,7 @@ class SNMPError(Exception): | |||||||||||||||
|
|
||||||||||||||||
|
|
||||||||||||||||
| @dataclass(kw_only=True) | ||||||||||||||||
| class SNMPServer(Driver): | ||||||||||||||||
| class SNMPServer(PowerInterface, Driver): | ||||||||||||||||
| """SNMP Power Control Driver""" | ||||||||||||||||
|
Comment on lines
40
to
42
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Rename Line 41 makes this a power driver; the naming rule requires a ♻️ Suggested rename (update references accordingly)-class SNMPServer(PowerInterface, Driver):
+class SNMPPower(PowerInterface, Driver):As per coding guidelines, power driver class names should end with 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would make it backwards incompatible and break users
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🧠 Learnings used |
||||||||||||||||
|
|
||||||||||||||||
| host: str = field() | ||||||||||||||||
|
|
@@ -113,11 +115,7 @@ def _setup_snmp(self): | |||||||||||||||
|
|
||||||||||||||||
| return snmp_engine | ||||||||||||||||
|
|
||||||||||||||||
| @classmethod | ||||||||||||||||
| def client(cls) -> str: | ||||||||||||||||
| return "jumpstarter_driver_snmp.client.SNMPServerClient" | ||||||||||||||||
|
|
||||||||||||||||
| def _create_snmp_callback(self, result: Dict[str, Any], response_received: asyncio.Event): | ||||||||||||||||
| def _create_snmp_callback(self, result: Dict[str, Any], response_received: Event): | ||||||||||||||||
| def callback(snmpEngine, sendRequestHandle, errorIndication, errorStatus, errorIndex, varBinds, cbCtx): | ||||||||||||||||
| self.logger.debug(f"Callback {errorIndication} {errorStatus} {errorIndex} {varBinds}") | ||||||||||||||||
| if errorIndication: | ||||||||||||||||
|
|
@@ -138,29 +136,17 @@ def callback(snmpEngine, sendRequestHandle, errorIndication, errorStatus, errorI | |||||||||||||||
|
|
||||||||||||||||
| return callback | ||||||||||||||||
|
|
||||||||||||||||
| def _setup_event_loop(self) -> Tuple[asyncio.AbstractEventLoop, bool]: | ||||||||||||||||
| try: | ||||||||||||||||
| loop = asyncio.get_running_loop() | ||||||||||||||||
| return loop, False | ||||||||||||||||
| except RuntimeError: | ||||||||||||||||
| loop = asyncio.new_event_loop() | ||||||||||||||||
| asyncio.set_event_loop(loop) | ||||||||||||||||
| return loop, True | ||||||||||||||||
|
|
||||||||||||||||
| async def _run_snmp_dispatcher(self, snmp_engine: engine.SnmpEngine, response_received: asyncio.Event): | ||||||||||||||||
| async def _run_snmp_dispatcher(self, snmp_engine: engine.SnmpEngine, response_received: Event): | ||||||||||||||||
| snmp_engine.open_dispatcher() | ||||||||||||||||
| await response_received.wait() | ||||||||||||||||
| snmp_engine.close_dispatcher() | ||||||||||||||||
|
|
||||||||||||||||
| def _snmp_set(self, state: PowerState): | ||||||||||||||||
| async def _snmp_set(self, state: PowerState): | ||||||||||||||||
| result = {"success": False, "error": None} | ||||||||||||||||
| response_received = asyncio.Event() | ||||||||||||||||
| loop = None | ||||||||||||||||
| created_loop = False | ||||||||||||||||
| response_received = Event() | ||||||||||||||||
|
|
||||||||||||||||
| try: | ||||||||||||||||
| self.logger.info(f"Sending power {state.name} command to {self.host}") | ||||||||||||||||
| loop, created_loop = self._setup_event_loop() | ||||||||||||||||
| snmp_engine = self._setup_snmp() | ||||||||||||||||
| callback = self._create_snmp_callback(result, response_received) | ||||||||||||||||
| cmdgen.SetCommandGenerator().send_varbinds( | ||||||||||||||||
|
|
@@ -172,10 +158,10 @@ def _snmp_set(self, state: PowerState): | |||||||||||||||
| callback, | ||||||||||||||||
| ) | ||||||||||||||||
|
|
||||||||||||||||
| dispatcher_task = loop.create_task(self._run_snmp_dispatcher(snmp_engine, response_received)) | ||||||||||||||||
| try: | ||||||||||||||||
| loop.run_until_complete(asyncio.wait_for(dispatcher_task, self.timeout)) | ||||||||||||||||
| except asyncio.TimeoutError: | ||||||||||||||||
| with fail_after(self.timeout): | ||||||||||||||||
| await self._run_snmp_dispatcher(snmp_engine, response_received) | ||||||||||||||||
| except TimeoutError: | ||||||||||||||||
| self.logger.warning(f"SNMP operation timed out after {self.timeout} seconds") | ||||||||||||||||
| result["error"] = "SNMP operation timed out" | ||||||||||||||||
|
|
||||||||||||||||
|
|
@@ -188,19 +174,20 @@ def _snmp_set(self, state: PowerState): | |||||||||||||||
| error_msg = f"SNMP set failed: {str(e)}" | ||||||||||||||||
| self.logger.error(error_msg) | ||||||||||||||||
| raise SNMPError(error_msg) from e | ||||||||||||||||
| finally: | ||||||||||||||||
| if created_loop and loop: | ||||||||||||||||
| loop.close() | ||||||||||||||||
|
|
||||||||||||||||
| @export | ||||||||||||||||
| def on(self): | ||||||||||||||||
| async def on(self): | ||||||||||||||||
| """Turn power on""" | ||||||||||||||||
| return self._snmp_set(PowerState.ON) | ||||||||||||||||
| return await self._snmp_set(PowerState.ON) | ||||||||||||||||
|
|
||||||||||||||||
| @export | ||||||||||||||||
| def off(self): | ||||||||||||||||
| async def off(self): | ||||||||||||||||
| """Turn power off""" | ||||||||||||||||
| return self._snmp_set(PowerState.OFF) | ||||||||||||||||
| return await self._snmp_set(PowerState.OFF) | ||||||||||||||||
|
|
||||||||||||||||
| @export | ||||||||||||||||
| async def read(self) -> AsyncGenerator[PowerReading, None]: | ||||||||||||||||
| raise NotImplementedError | ||||||||||||||||
|
Comment on lines
+188
to
+190
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type mismatch: function is not an async generator without The return type declares 🐛 Proposed fix `@export`
async def read(self) -> AsyncGenerator[PowerReading, None]:
raise NotImplementedError
+ yield # Makes this an async generatorAlternatively, if intentional that this driver doesn't support power readings: -@export
-async def read(self) -> AsyncGenerator[PowerReading, None]:
- raise NotImplementedError
+async def read(self) -> AsyncGenerator[PowerReading, None]:
+ raise NotImplementedError("SNMP power reading is not supported")
+ yield # Required to make this an async generator📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| def close(self): | ||||||||||||||||
| """No cleanup needed since engines are created per operation""" | ||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ dev = [ | |
| "pytest-cov>=5.0.0", | ||
| "requests>=2.32.3", | ||
| "rpmfile>=2.1.0", | ||
| "zstandard>=0.23.0", | ||
| "zstandard>=0.25.0", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Verify zstandard 0.25.0 exists on PyPI
echo "=== Checking zstandard 0.25.0 on PyPI ==="
curl -s https://pypi.org/pypi/zstandard/json | jq -r '.releases | keys[]' | sort -V | tail -10
echo -e "\n=== Checking for 0.25.0 specifically ==="
curl -s https://pypi.org/pypi/zstandard/json | jq -r '.releases | keys[]' | grep "^0\.25\.0$"Repository: jumpstarter-dev/jumpstarter Length of output: 231 🏁 Script executed: #!/bin/bash
# Examine the full pyproject.toml file
echo "=== Full pyproject.toml content ==="
cat -n packages/jumpstarter-driver-uboot/pyproject.tomlRepository: jumpstarter-dev/jumpstarter Length of output: 1535 🏁 Script executed: #!/bin/bash
# Check for entry point registration
echo "=== Checking for jumpstarter.drivers entry point ==="
grep -n "jumpstarter.drivers" packages/jumpstarter-driver-uboot/pyproject.tomlRepository: jumpstarter-dev/jumpstarter Length of output: 123 Add missing entry point registration for the driver. The zstandard version bump to 0.25.0 is valid. However, this driver package is missing the required 🤖 Prompt for AI Agents |
||
| ] | ||
|
|
||
| [tool.hatch.metadata.hooks.vcs.urls] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,7 +10,7 @@ authors = [ | |
| readme = "README.md" | ||
| license = "Apache-2.0" | ||
| requires-python = ">=3.11" | ||
| dependencies = ["jumpstarter", "pillow>=10.4.0"] | ||
| dependencies = ["jumpstarter", "pillow>=12.1.0"] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: cat packages/jumpstarter-driver-ustreamer/pyproject.tomlRepository: jumpstarter-dev/jumpstarter Length of output: 958 🌐 Web query:
💡 Result: Pillow 12.1.0 requires Python >= 3.10 and is marked as supporting Python 3.10–3.14. [1][2] Sources:
🏁 Script executed: # Check if other jumpstarter-driver packages have entry points
fd -t f "pyproject.toml" packages/jumpstarter-driver-* | head -3 | xargs -I {} sh -c 'echo "=== {} ===" && grep -A 5 "entry-points\|jumpstarter.drivers" {}'Repository: jumpstarter-dev/jumpstarter Length of output: 718 🏁 Script executed: # Check full content of ustreamer pyproject.toml for entry-points
grep -n "entry-points" packages/jumpstarter-driver-ustreamer/pyproject.toml || echo "No entry-points found"Repository: jumpstarter-dev/jumpstarter Length of output: 92 Missing required Driver packages must register via the The package otherwise complies with driver naming conventions and includes the required 🤖 Prompt for AI Agents |
||
|
|
||
| [dependency-groups] | ||
| dev = ["pytest>=8.3.2", "pytest-cov>=5.0.0"] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,7 +15,7 @@ dependencies = [ | |
| "pyyaml>=6.0.2", | ||
| "anyio>=4.4.0,!=4.6.2", | ||
| "aiohttp>=3.10.5", | ||
| "pydantic>=2.8.2", | ||
| "pydantic>=2.12.5", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: find . -path "*/packages/jumpstarter/pyproject.toml" -type fRepository: jumpstarter-dev/jumpstarter Length of output: 108 🏁 Script executed: cat -n packages/jumpstarter/pyproject.toml | head -40Repository: jumpstarter-dev/jumpstarter Length of output: 1330 🌐 Web query:
💡 Result: Yes — pydantic-settings 2.9.1 is compatible with Pydantic 2.12.5. pydantic-settings 2.9.1 declares a dependency of pydantic >= 2.7.0, so Pydantic 2.12.5 satisfies that requirement. [1][2] Sources:
🌐 Web query:
💡 Result: Short answer: No — pydantic-settings 2.9.1 (released May 23, 2025) does not include official Python 3.14 support. Upgrade to a pydantic release with 3.14 support (pydantic v2.12+ adds initial Python 3.14 support). [1][2] Sources:
🌐 Web query:
💡 Result: Yes — the Pydantic 2.12 series adds initial Python 3.14 support, and 2.12.5 is part of that release line. See the v2.12 release notes (Python 3.14 support highlighted) and the PyPI release for 2.12.5. [1][2] Sources: Update pydantic-settings to a version with Python 3.14 support. Pydantic 2.12.5 is compatible with pydantic-settings 2.9.1 (which requires pydantic ≥2.7.0). However, pydantic-settings 2.9.1 does not support Python 3.14, while pydantic 2.12.5 does. Since the project declares 🤖 Prompt for AI Agents |
||
| "xdg-base-dirs>=6.0.2", | ||
| "pydantic-settings>=2.9.1", | ||
| "rich>=14.0.0", | ||
|
|
||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: jumpstarter-dev/jumpstarter
Length of output: 198
🏁 Script executed:
Repository: jumpstarter-dev/jumpstarter
Length of output: 198
🌐 Web query:
jumpstarter SNMP driver anyio dependency pyproject.toml💡 Result:
Short answer: jumpstarter-driver-snmp does not list anyio as a dependency — its published package depends on click, jumpstarter-driver-power, pysnmp and jumpstarter. If you need anyio for your project, add it to your pyproject.toml dependencies (example below).
Sources: PyPI jumpstarter-driver-snmp (dependencies list). [1] AnyIO project page for package name. [2]
Example pyproject.toml snippet to add anyio (adjust version constraint as you need):
[project]
dependencies = [
"jumpstarter-driver-snmp",
"anyio>=3.0", # or anyio>=4.0 / >=4.11 depending on your target
]
References:
Add
anyioas a runtime dependency inpackages/jumpstarter-driver-snmp/pyproject.toml.Lines 7–8 import
anyio(Event,fail_after), but the published SNMP driver package does not declareanyioin its dependencies. This will cause anImportErrorat runtime. Addanyioto thedependenciessection of the package manifest.🤖 Prompt for AI Agents