feat(integrations): LabelData schema + Snipe-IT lookup client#51
Conversation
Introduces the app-agnostic LabelData value object (frozen Pydantic model) and the SnipeITClient that maps Snipe-IT asset payloads to LabelData. Adds 9 tests (4 schema, 5 client); total suite: 81 passed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ion, document HTTP error policy - Guard against missing 'id' in Snipe-IT responses: raise ValueError before constructing LabelData so callers never receive a silently broken .../hardware/None QR URL. - Add regression test asserting ValueError with match on 'missing required field 'id'' for a 200 response that omits the id field. - Tighten test_label_data_is_frozen: switch from bare pydantic.ValidationError to pydantic_core.ValidationError with match='frozen_instance' so the test fails if Pydantic ever changes the freeze behaviour. - Add one-line comment above raise_for_status() documenting the HTTP error propagation policy for future maintainers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes the foundational data structures and the first integration client for the label printing system. By defining a frozen Pydantic schema, it ensures that downstream components receive a consistent, app-agnostic representation of entities regardless of the source. The addition of the Snipe-IT client provides a robust, authenticated interface for asset lookups, complete with proper error handling and regression testing. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Adds an integration boundary for upstream lookups by introducing a frozen LabelData value object and a first lookup client implementation for Snipe-IT asset tags, with unit tests to validate the mapping and error behavior.
Changes:
- Introduce
LabelDataPydantic model to standardize what lookup clients return. - Add
SnipeITClientasync REST client that maps/hardware/bytag/{asset_tag}responses intoLabelData. - Add unit tests for both the schema behavior and the Snipe-IT client behavior.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/app/schemas/label_data.py | Adds the LabelData domain schema used across integrations. |
| backend/app/schemas/init.py | Introduces schemas package docstring. |
| backend/app/services/snipeit_client.py | Adds async Snipe-IT lookup client that emits LabelData. |
| backend/tests/unit/schemas/test_label_data.py | Adds unit tests for LabelData construction and “frozen” behavior. |
| backend/tests/unit/services/test_snipeit_client.py | Adds unit tests for Snipe-IT lookup success and error cases. |
| primary_id: str | ||
| qr_payload: str | ||
| source_app: str | ||
| secondary: list[str] = Field(default_factory=list) |
| # With frozen=True we cannot append, but we can still verify the lists are distinct objects. | ||
| assert a.secondary is not b.secondary |
|
|
||
| async def lookup(self, asset_tag: str) -> LabelData: | ||
| """Return LabelData for `asset_tag`, or raise SnipeITNotFoundError.""" | ||
| url = f"{self._base_url}/api/v1/hardware/bytag/{asset_tag}" |
| headers = { | ||
| "Authorization": f"Bearer {self._api_key}", | ||
| "Accept": "application/json", | ||
| } |
|
|
||
| async def lookup(self, asset_tag: str) -> LabelData: | ||
| """Return LabelData for `asset_tag`, or raise SnipeITNotFoundError.""" | ||
| url = f"{self._base_url}/api/v1/hardware/bytag/{asset_tag}" | ||
| headers = { | ||
| "Authorization": f"Bearer {self._api_key}", | ||
| "Accept": "application/json", | ||
| } | ||
| async with httpx.AsyncClient(timeout=self._timeout) as client: | ||
| response = await client.get(url, headers=headers) |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the LabelData schema and a SnipeITClient for asset lookups, along with corresponding unit tests. Feedback includes a requirement to URL-encode the asset_tag to prevent broken URL structures and a recommendation to reuse the httpx.AsyncClient for better performance. Additionally, the use of Any in the response payload mapping should be replaced with a TypedDict to comply with the project's type safety standards.
|
|
||
| async def lookup(self, asset_tag: str) -> LabelData: | ||
| """Return LabelData for `asset_tag`, or raise SnipeITNotFoundError.""" | ||
| url = f"{self._base_url}/api/v1/hardware/bytag/{asset_tag}" |
There was a problem hiding this comment.
The asset_tag should be URL-encoded before being interpolated into the path. Snipe-IT asset tags can contain spaces, hashes, or other special characters that would break the URL structure or be interpreted as fragments if not escaped.
from urllib.parse import quote
url = f"{self._base_url}/api/v1/hardware/bytag/{quote(asset_tag)}"| async with httpx.AsyncClient(timeout=self._timeout) as client: | ||
| response = await client.get(url, headers=headers) |
There was a problem hiding this comment.
Creating a new httpx.AsyncClient for every request is inefficient as it prevents connection pooling and adds significant overhead for each lookup. Consider accepting a shared client in the constructor or the lookup method to improve performance and resource utilization.
| # decide whether to treat them as configuration errors vs transient failures. | ||
| response.raise_for_status() | ||
|
|
||
| payload: dict[str, Any] = response.json() |
There was a problem hiding this comment.
The use of Any in dict[str, Any] violates the repository's type safety policy (Rule 21). Using a TypedDict for the Snipe-IT response payload would provide better type safety and eliminate the need for Any throughout the mapping logic.
References
- Type safety. mypy --strict on app/. Flag new Any introductions. (link)
…+ auth-header test - LabelData.secondary changed from list[str] to tuple[str, ...] to enforce true immutability under frozen=True (list.append() was still possible before) - Pydantic v2 coerces list inputs to tuple automatically; callers unchanged - _payload_to_label now passes tuple(secondary) explicitly for mypy clarity - urllib.parse.quote(asset_tag, safe="") prevents URL breakage on tags with special characters (/, ?, space, #) - Added TODO(phase6) comment on lookup() for future httpx.AsyncClient pooling - Schema tests: replaced default-distinct identity test with immutability test (tuple.append raises AttributeError); updated secondary == () comparisons - New tests: test_lookup_url_encodes_asset_tag, test_lookup_sends_bearer_auth_header Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ssages + LRU-cached font loader - Fix A: TemplateSchema.elements changed from list to tuple[LayoutElement, ...] so frozen=True actually prevents mutation (same pattern as LabelData.secondary fix in PR #51). Pydantic v2 coerces list inputs automatically; callers are unaffected. - Fix B: LayoutElement validator messages now use !r to show the actual rejected value (None, 0, -5 etc.) instead of the generic "got None or 0" string. - Fix C: _load_font promoted from a per-call static method to a module-level @functools.lru_cache(maxsize=32) function, eliminating repeated disk I/O for multi-element templates. - Add 4 regression tests: immutability, negative size, negative font_size, cache identity. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
## 0.3.0 (2026-05-12) * feat(config): pydantic-settings module with env-driven runtime configuration (#45) ([878e9e0](878e9e0)), closes [#45](#45) * feat(integrations): AppLookupService aggregator — Phase 3 complete (#53) ([222bef4](222bef4)), closes [#53](#53) * feat(integrations): Grocy + Spoolman lookup clients with shared NotFoundError base (#52) ([b1c9c3c](b1c9c3c)), closes [#52](#52) * feat(integrations): LabelData schema + Snipe-IT lookup client (#51) ([3bc180f](3bc180f)), closes [#51](#51) * feat(label-renderer): Template schema + Pillow/qrcode renderer for 1-bit label bitmaps (#54) ([fb77028](fb77028)), closes [#54](#54) * feat(printer-models): Brother PT-Series TapeRegistry with TZe and heat-shrink specs (#47) ([7526019](7526019)), closes [#47](#47) * feat(printer-models): Job lifecycle FSM with explicit state machine (#49) ([1a8c40e](1a8c40e)), closes [#49](#49) * feat(printer-models): PrinterModel Protocol + ModelRegistry for plugin discovery (#48) ([2ae0e09](2ae0e09)), closes [#48](#48) * feat(printer-models): PrintQueue worker with pause/resume/cancel/retry (#50) ([dfdf6fe](dfdf6fe)), closes [#50](#50) [skip ci]
Summary
Phase 3 PR C1 of 3. Adds:
LabelDatafrozen Pydantic value object — what every lookup client (Snipe-IT now, Grocy + Spoolman in PR C2, OpenFoodFacts later) emits.asset_tagviaAuthorization: Bearer <api_key>.The Label Renderer (Phase 4) and Aggregator (PR C3) consume
LabelData. No upstream service shape leaks past the client layer.What's in this PR
backend/app/schemas/label_data.pyLabelData(BaseModel)withConfigDict(frozen=True).title,primary_id,qr_payload,source_app,secondary: list[str].default_factory=listenforces immutable instances with per-instance default secondary lists (not shared mutable defaults).backend/app/services/snipeit_client.pySnipeITClient(*, base_url, api_key, timeout=5.0)— keyword-only.base_url.rstrip("/")once at construction, never on every call.async lookup(asset_tag) -> LabelData:/api/v1/hardware/bytag/{asset_tag}with Bearer auth.SnipeITNotFoundError.httpx.HTTPStatusError(documented in inline comment)._payload_to_label()maps to LabelData.SnipeITNotFoundError(Exception)— N818 suffixError._payload_to_labelraisesValueErrorifidis missing in the response — avoids silently producing.../hardware/Nonein the QR URL.backend/tests/unit/schemas/test_label_data.py— 4 testspydantic_core.ValidationErrorwithmatch="frozen_instance").backend/tests/unit/services/test_snipeit_client.py— 6 testsS/N: None).base_urlstripped from QR URL.httpx.HTTPStatusError(no swallowing).idfield →ValueError(catches silent broken URL).What's NOT in this PR
AppLookupServiceaggregator — PR C3.Test plan
pytest -q→ 82/82 (72 prior + 4 schema + 6 client).ruff format --check .clean.ruff check .clean.mypy app/(strict) clean.snipe-it.example(RFC 2606 reserved) — privacy clean.Review history (subagent-driven)
7549935— initial commit.payload.get('id')silent broken URL, loose frozen-test exception match, missing comment onraise_for_status()policy.bae14ff—id-guard with ValueError + new test, tightened frozen exception match (pydantic_core.ValidationError+match="frozen_instance"—PydanticFrozenInstanceErroris not a public export in pydantic-core 2.46.4, fallback achieves same intent), one-line comment documenting HTTP error policy.Linked plan
docs/superpowers/plans/2026-05-11-label-printer-hub.mdTasks 3.1 + 3.2.PR C2 (Grocy + Spoolman clients) and PR C3 (AppLookupService aggregator) follow this branch.