Skip to content

feat(integrations): Grocy + Spoolman lookup clients with shared NotFoundError base#52

Merged
strausmann merged 2 commits into
mainfrom
feat/app-lookup-grocy-spoolman
May 11, 2026
Merged

feat(integrations): Grocy + Spoolman lookup clients with shared NotFoundError base#52
strausmann merged 2 commits into
mainfrom
feat/app-lookup-grocy-spoolman

Conversation

@strausmann
Copy link
Copy Markdown
Owner

Summary

Phase 3 PR C2 of 3. Adds two more REST clients for the App-Lookup-Service:

  • GrocyClient — product lookup via GROCY-API-KEY header. Special-cases Grocy's quirk of returning HTTP 400 (not 404) for missing products.
  • SpoolmanClient — filament-spool lookup; no auth (Spoolman is intended for trusted-network use).

Both follow the defensive patterns established for SnipeITClient (PR #51): keyword-only init, normalised base_url, URL-encoded id, ValueError on missing id in response, tuple secondary, documented HTTP error policy.

Also introduces the shared AppLookupNotFoundError base so PR C3 (Aggregator) can catch any client's not-found in one except clause. All three concrete *NotFoundError classes (incl. the already-merged Snipe-IT one) now inherit from it.

What's in this PR

backend/app/services/errors.py (new)

  • AppLookupNotFoundError(Exception) — shared base for all lookup clients.

backend/app/services/grocy_client.py

  • GrocyClient(*, base_url, api_key, timeout=5.0).
  • Endpoint /api/objects/products/{quote(id)} with header GROCY-API-KEY: <key> (NOT Bearer).
  • 400 or 404GrocyNotFoundError (Grocy's not-found quirk).
  • 5xx → raw httpx.HTTPStatusError.
  • Guard: id missing → ValueError.

backend/app/services/spoolman_client.py

  • SpoolmanClient(*, base_url, timeout=5.0) — no api_key parameter, no auth header.
  • Endpoint /api/v1/spool/{quote(id)}.
  • Title composed from filament.vendor.name + filament.name (graceful "Unknown" fallback).
  • primary_id = f"#{id}".
  • Secondary line: "{grams}g remaining" rounded half-up (banker's rounding would print 850 for 850.5 — commented inline).

backend/app/services/snipeit_client.py

  • One-line change: SnipeITNotFoundError(Exception)SnipeITNotFoundError(AppLookupNotFoundError). Transparent to all existing callers.

Tests (+19, total 103)

  • 8 Grocy: happy path, 400-not-found, 404-not-found (forward-compat), trailing slash stripped, URL encoding, 5xx surfaces raw, missing-id guard, GROCY-API-KEY header sent (and no Authorization).
  • 9 Spoolman: happy path with 850.5→"851g remaining", 404, missing remaining weight, missing vendor.name, null filament, trailing slash, URL encoding, 5xx, missing-id guard, no-auth-header assertion.
  • 1 cross-cutting: test_not_found_error_is_app_lookup_not_found verifies all three concrete NotFoundError types inherit from the shared base.
  • +1 tightened: test_lookup_url_encodes_spool_id now asserts the response (was a no-op).

What's NOT in this PR

  • AppLookupService aggregator — PR C3.
  • FastAPI endpoint wiring — Phase 6.
  • httpx.AsyncClient connection pooling — TODO(phase6) comment on each client.

Test plan

  • pytest -q → 103/103.
  • ruff format --check . clean.
  • ruff check . clean.
  • mypy app/ (strict) clean — 21 source files.
  • All test URLs use .example (RFC 2606 reserved).

Review history (subagent-driven)

  1. Implementer 51ab508 — initial Grocy + Spoolman clients (17 new tests).
  2. Spec compliance: ✅ — all 17 tests + signatures + defensive patterns present.
  3. Code quality: APPROVED_WITH_NITS — flagged absence of shared base exception (Important, would force C3 to touch already-merged files), missing rounding-rationale comment, loose test_lookup_url_encodes_spool_id, no test for null filament.
  4. Fix commit 31ff882 — introduced app/services/errors.py + cross-cutting inheritance fix touching the already-merged Snipe-IT client too, inline rounding comment, tightened test, null-filament test added.

Linked plan

docs/superpowers/plans/2026-05-11-label-printer-hub.md Tasks 3.3 + 3.4.

PR C3 (AppLookupService aggregator + per-app routing) builds on this branch.

strausmann and others added 2 commits May 11, 2026 10:19
…-guards

- GrocyClient: GROCY-API-KEY header auth, maps both 400 and 404 to
  GrocyNotFoundError (Grocy quirk), raises ValueError on missing 'id'
- SpoolmanClient: no-auth trusted-network client, '#'-prefixed primary_id,
  remaining_weight formatted with round-half-up, raises ValueError on
  missing 'id'
- Both clients: keyword-only __init__, URL-encoded ids, base_url trailing-
  slash strip, TODO(phase6) httpx.AsyncClient pooling comment
- 17 new tests (8 Grocy + 9 Spoolman), 101 total pass

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ng comments and tighter tests

- Introduce backend/app/services/errors.py with AppLookupNotFoundError base
- SnipeITNotFoundError, GrocyNotFoundError, SpoolmanNotFoundError all inherit
  from the base so callers can catch any client's not-found in one clause
- Add rounding comment to spoolman_client.py explaining math.floor(x+0.5)
  vs banker's rounding in round() / f"{x:.0f}"
- Tighten test_lookup_url_encodes_spool_id to assert source_app and primary_id
- Add test_lookup_spool_with_null_filament for fully-null filament path
- Add test_not_found_error_is_app_lookup_not_found cross-client inheritance check

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 11, 2026 10:26
@gemini-code-assist
Copy link
Copy Markdown

Summary of Changes

Hello, 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 expands the App-Lookup-Service by introducing two new REST clients for Grocy and Spoolman. It establishes a shared exception base to simplify future aggregation logic and ensures consistent defensive programming practices across all integration clients. The changes include robust test coverage for various edge cases, including API-specific error handling and authentication requirements.

Highlights

  • New Integration Clients: Added GrocyClient and SpoolmanClient to support product and filament-spool lookups, implementing defensive patterns like URL encoding and strict response validation.
  • Shared Exception Hierarchy: Introduced AppLookupNotFoundError as a base class for all lookup-related exceptions, allowing for unified error handling across different service clients.
  • API-Specific Quirks: Handled Grocy's non-standard 400-error response for missing items and implemented custom rounding logic for Spoolman weight data.
Using Gemini Code Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Grocy and Spoolman lookup clients to the backend “App-Lookup” layer, plus a shared AppLookupNotFoundError base class so the upcoming aggregator can catch all client “not found” cases uniformly.

Changes:

  • Introduces GrocyClient and SpoolmanClient async REST clients that emit app-agnostic LabelData, including consistent URL normalization, URL-encoding of IDs, and explicit HTTP error mapping.
  • Adds AppLookupNotFoundError and updates SnipeITNotFoundError to inherit from it for cross-client not-found handling.
  • Adds/extends unit tests covering happy paths, not-found mapping, URL encoding, header behavior, and defensive “missing id” guards.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
backend/app/services/errors.py Adds shared AppLookupNotFoundError base exception for lookup clients.
backend/app/services/grocy_client.py Implements GrocyClient with Grocy-specific auth header + 400/404→not-found mapping.
backend/app/services/spoolman_client.py Implements SpoolmanClient with title composition + half-up rounding for remaining grams.
backend/app/services/snipeit_client.py Makes SnipeITNotFoundError inherit from AppLookupNotFoundError.
backend/tests/unit/services/test_grocy_client.py Adds Grocy client tests for success, not-found behavior, encoding, headers, and error surfacing.
backend/tests/unit/services/test_spoolman_client.py Adds Spoolman client tests for success, rounding, not-found behavior, encoding, and no-auth headers.
backend/tests/unit/services/test_snipeit_client.py Adds a cross-cutting test asserting all concrete not-found errors share the common base.

@strausmann strausmann merged commit b1c9c3c into main May 11, 2026
13 checks passed
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces new service clients for Grocy and Spoolman to facilitate entity lookups for label printing. It also establishes a shared exception hierarchy by introducing AppLookupNotFoundError, which existing and new clients now inherit from. The implementation correctly utilizes httpx for asynchronous requests and includes comprehensive unit tests for the new clients and exception inheritance. I have no feedback to provide as the changes align with the project's architectural patterns and style guidelines.

github-actions Bot pushed a commit that referenced this pull request May 12, 2026
## 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]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants