fix: BrotherQLBackend.preflight_check (Phase 1i hotfix)#99
Conversation
…k 8b) Live-Bug aus Phase 1i Smoke-Test auf hhdocker03: AttributeError: 'BrotherQLBackend' object has no attribute 'preflight_check' print_service.py:90 ruft backend.preflight_check() vor jedem Job. PTouch implementiert es, BrotherQLBackend (Phase 1i Task 8) hatte die Methode nicht — fehlte im Task 8 Spec. QL-820NWB nutzt dieselbe SNMP-Probe wie PT-Series (Printer MIB Standard). 3 Tests: success, tape_empty, snmp_offline. Refs Phase 1i Smoke-Bug
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! Dieser Pull Request behebt einen Laufzeitfehler, der während des Smoke-Tests in Phase 1i auftrat, bei dem die Methode 'preflight_check' im BrotherQLBackend fehlte. Durch die Implementierung dieser Methode wird sichergestellt, dass der Drucker vor jedem Job korrekt abgefragt wird, was die Stabilität und Fehlererkennung des Druckdienstes verbessert. 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
This PR hotfixes Phase 1i by adding the missing BrotherQLBackend.preflight_check() method so PrintService can run the SNMP preflight step before every print job, matching the existing PT-Series behavior.
Changes:
- Implement
BrotherQLBackend.preflight_check()usingquery_preflight()and map SNMP error flags toTapeEmptyError/PrinterCoverOpenError/PrinterOfflineError. - Add unit tests covering success, tape-empty, and SNMP-offline scenarios for the new preflight method.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/app/printer_backends/brother_ql_backend.py | Adds SNMP-based preflight_check() to the QL backend and raises the same readiness errors as PT-Series. |
| backend/tests/unit/printer_backends/test_brother_ql_backend.py | Adds unit tests validating the QL preflight behavior and error mapping. |
| ) | ||
|
|
||
| monkeypatch.setattr("app.printer_backends.brother_ql_backend.query_preflight", fake_preflight) | ||
| backend = BrotherQLBackend(host="172.16.51.213", model_id="QL-820NWB") |
| except SnmpQueryError as exc: | ||
| raise PrinterOfflineError(f"preflight SNMP failed: {exc}") from exc |
There was a problem hiding this comment.
Code Review
This pull request implements SNMP-based preflight checks for the Brother QL printer backend, raising specific exceptions for offline, empty tape, or open cover states, and adds corresponding unit tests. The review feedback correctly identifies a privacy violation in the test code where a hardcoded LAN IP address is used, and suggests replacing it with an RFC 5737 documentation IP address in compliance with the repository's style guide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ) | ||
|
|
||
| monkeypatch.setattr("app.printer_backends.brother_ql_backend.query_preflight", fake_preflight) | ||
| backend = BrotherQLBackend(host="172.16.51.213", model_id="QL-820NWB") |
There was a problem hiding this comment.
Avoid using real LAN IP addresses (such as 172.16.51.213) in test code to prevent internal network details from being deducible. Instead, use RFC 5737 documentation IP addresses (e.g., 192.0.2.11).
| backend = BrotherQLBackend(host="172.16.51.213", model_id="QL-820NWB") | |
| backend = BrotherQLBackend(host="192.0.2.11", model_id="QL-820NWB") |
References
- Privacy violations. Flag any hardcoded LAN IPs, real hostnames, real domains, real tokens, or PII. The maintainer's network must not be deducible from this repository. (link)
- Use RFC 5737 documentation IPs and 'example.com' placeholders instead of real LAN IPs, hostnames, or domains in documentation and code to maintain privacy.
Add __all__ to snmp_helper.py so SnmpQueryError (imported from app.printer_backends.exceptions) is explicitly re-exported. mypy strict no_implicit_reexport requires this for imports in brother_ql_backend.py. Replace real HomeLab IP 172.16.51.213 in test_brother_ql_backend.py:99 with RFC 5737 documentation address 192.0.2.11, consistent with PR #98 commit 541502e. Refs PR #98 review-followup
…untime_printers PR#98-Gemini: Replace session.commit() inside the upsert loop with session.flush() so all printer rows land in a single atomic transaction. The single commit() at the end of the loop remains. PR#98-Copilot: Add slug-collision detection before INSERT. When a DB row already exists with the same slug but a different UUID (e.g. after model/host/port change in printers.yaml), the old row is deleted via delete+flush and the new deterministic UUID row is inserted. A WARNING is logged to make the migration visible in startup logs. Add 3 new integration tests covering: - Same-UUID idempotent update (flush semantics) - Slug-collision-different-UUID migration (warn+replace) - Multi-printer transaction atomicity Refs PR #98 review-followup
…se behavior PR#98-Copilot: The previous docstring claimed True is interpreted as 'no-cut-between' on QL-Series. The actual implementation in batch_dispatch.py forces half_cut=False when backend.half_cut_supported is False and logs a warning — there is no 'no-cut-between' semantics. Update the Field description to reflect the real behavior. Refs PR #98 review-followup
…iew-png drift PR#98-Copilot C.1: config.py — clarify that extra="forbid" rejects unknown constructor kwargs only, NOT unknown env vars. pydantic-settings silently ignores unknown env vars regardless of this setting. PR#98-Copilot C.2: label_renderer.py — replace stale '600 px at 300 DPI' comment with a proper module-level docstring that decouples the canvas from any specific DPI (PT 180 DPI, QL 300 DPI). PR#98-Copilot C.3: test_openapi_completeness.py — align docstring lower bound (was 23, now 28) with the actual assertion (>= 28). PR#98-Copilot C.4: docs/research/2026-06-02-pt750w-layout-diagnose.md — fix preview endpoint URL from /preview.png to /preview-png to match the implemented route. Also: fix RUF022 — sort __all__ in snmp_helper.py alphabetically (query_live_status before query_loaded_tape_mm); format test_lifespan_printer_upsert.py. Refs PR #98 review-followup
…ision check Replace `Printer.slug == cfg.slug` with `col(Printer.slug) == cfg.slug` in the slug-collision SELECT. SQLModel/SQLAlchemy requires col() to get proper ColumnElement typing; without it mypy strict reports arg-type error (Argument 1 to "where" has incompatible type "bool"). Pattern consistent with app/repositories/printers.py line 41. Refs PR #98 review-followup
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #99 +/- ##
==========================================
+ Coverage 89.51% 89.59% +0.08%
==========================================
Files 95 95
Lines 4291 4316 +25
Branches 365 368 +3
==========================================
+ Hits 3841 3867 +26
+ Misses 359 357 -2
- Partials 91 92 +1
... and 3 files with indirect coverage changes
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
## 0.9.0 (2026-06-05) * feat: Phase 1i — Label-Quality + Multi-Printer + Samla + Preview (#98) ([a33dda9](a33dda9)), closes [#98](#98) * feat: Phase 1k.2 Multi-Label-Batch via ptouch.print_multi (#106) ([a9726a9](a9726a9)), closes [#106](#106) [#101](#101) [#103](#103) * feat(api): GET /api/templates/{key}/preview-svg + Dashboard-Modal (Phase 1i D) ([c2cbf3c](c2cbf3c)) * feat(api): GET /api/templates/{key}/preview.png (Diagnose-Tool, Phase 1i A) ([27dd325](27dd325)), closes [#5](#5) * feat(batch): dispatch_batch refactor to atomic BatchJob + submit_batch_job (Phase 1k.2 Task 9) ([a2b6a98](a2b6a98)), closes [#102](#102) [#106](#106) * feat(batch): printer_slug + half_cut_override + BackendRouter-Lookup (Phase 1i C) ([0561504](0561504)), closes [#6](#6) * feat(brother_ql): print_images via default_print_images_loop (Phase 1k.2 Task 4) ([f56e44c](f56e44c)), closes [#102](#102) * feat(config): Clean-Break Settings + upsert_runtime_printers (Phase 1i H CA-1 7a) ([64c38b3](64c38b3)) * feat(lifespan): Multi-Printer-Wiring + service_for pro Drucker (Phase 1i H CA-1 7b) ([3661a4e](3661a4e)) * feat(mock): MockBackend.print_images via default_print_images_loop (Phase 1k.2 Task 5) ([4ebdbee](4ebdbee)), closes [#102](#102) * feat(printer_backends): BrotherQLBackend + QLSeriesModel (Phase 1i G) ([ad52009](ad52009)) * feat(printer_backends): default_print_images_loop helper (Phase 1k.2 Task 1) ([54e1123](54e1123)), closes [#102](#102) * feat(printer_backends): half_cut Parameter + half_cut_supported Capability (Phase 1i C) ([d41c246](d41c246)) * feat(printer_backends): PrinterBackend.print_images Protocol method (Phase 1k.2 Task 2) ([e93f945](e93f945)), closes [#102](#102) * feat(pt): _PTPQueuePrinter.print_images + bug-fix half_cut/last_page forwarding (Phase 1k.2 Task 6) ([15ef310](15ef310)), closes [#100](#100) [#100](#100) * feat(ptouch): print_images via ptouch.print_multi (Phase 1k.2 Task 3) ([c0d15f0](c0d15f0)), closes [#102](#102) * feat(ql): _QLQueuePrinter.print_images adapter (Phase 1k.2 Task 7) ([cb1e317](cb1e317)), closes [#102](#102) [#106](#106) * feat(queue): BatchJob + enqueue_batch + worker isinstance dispatch (Phase 1k.2 Task 8) ([c08da56](c08da56)), closes [#102](#102) [#106](#106) * feat(schemas): PrinterYAMLConfig + PrintersFile (Phase 1i H) ([7779a94](7779a94)) * feat(scripts): smoke_first_print_batch.py — manual 4-item batch hardware-smoke (Phase 1k.2 Task 12) ([d2ce28e](d2ce28e)), closes [#102](#102) * feat(services): BackendRouter mit slug->Backend-Mapping (Phase 1i H CA-2) ([a735a34](a735a34)) * feat(services): PrinterConfigLoader + printers.yaml.example (Phase 1i H) ([f065575](f065575)) * feat(templates): DPI-Fix + Samla-Templates (Phase 1i B) ([af7bce8](af7bce8)) * test(batch): end-to-end multi-label batch integration test (Phase 1k.2 Task 11) ([bcc1d04](bcc1d04)), closes [#102](#102) * test(queue): coverage for _process_batch error paths + pragma on _ptouch_print_multi (Phase 1k.2 Tas ([0b6250d](0b6250d)), closes [#106](#106) [#102](#102) * style: ruff format + lint cleanup (Phase 1i) ([6cffe4c](6cffe4c)) * style: ruff format Task 11 integration test (Phase 1k.2 cleanup) ([ad47f7e](ad47f7e)), closes [#102](#102) * fix: TTF font + last_page→feed (Phase 1i smoke-test bugs) (#100) ([3998575](3998575)), closes [#100](#100) * fix(api): preview-png statt preview.png (OpenAPI kebab-Konvention) ([b420722](b420722)) * fix(ci): mypy types + privacy scan (Phase 1i) ([541502e](541502e)) * fix(ci): SnmpQueryError __all__ export + privacy test IP ([99c1946](99c1946)), closes [#98](#98) [#98](#98) * fix(ci): UUID-Continuity-Runbook entfernen (privacy) ([b027cff](b027cff)) * fix(lifespan): col() for mypy-safe SQLModel where-clause in slug collision check ([6f9d840](6f9d840)), closes [#98](#98) * fix(lifespan): session.flush() + slug-collision-detection in upsert_runtime_printers ([e7d81e6](e7d81e6)), closes [#98](#98) * fix(print_service): forward auto_cut/high_resolution from request options (Phase 1k.2 Task 9 follow- ([51539b5](51539b5)), closes [#102](#102) * fix(printer_backends): BrotherQLBackend.preflight_check (Phase 1i Task 8b) ([dba9b6c](dba9b6c)) * fix(printer_backends): print_images stubs to fix Protocol isinstance regressions (Phase 1k.2 Task 2 ([5cbf955](5cbf955)), closes [#102](#102) * fix(ptouch): propagate last_page→feed to ptouch lib (short half-cut between batch items) ([05f780d](05f780d)) * fix(queue): align image payloads with active_jobs + strengthen Task 8 tests (Phase 1k.2 Task 8 follo ([c658e87](c658e87)), closes [#102](#102) [#106](#106) * fix(renderer): install fonts-dejavu-core in Dockerfile (font_size honored) ([9b0cd84](9b0cd84)) * fix(reviews): adresse Gemini + Copilot PR #100 findings ([9507432](9507432)), closes [#100](#100) [#100](#100) * fix(schemas): half_cut_override docstring matches actual force-to-false behavior ([a7b84b9](a7b84b9)), closes [#98](#98) * docs: pydantic-settings honesty + DPI-decouple + openapi range + preview-png drift ([3d8857e](3d8857e)), closes [PR#98-Copilot](https://github.com/PR/issues/98-Copilot) [PR#98-Copilot](https://github.com/PR/issues/98-Copilot) [PR#98-Copilot](https://github.com/PR/issues/98-Copilot) [PR#98-Copilot](https://github.com/PR/issues/98-Copilot) [#98](#98) * docs(plan): adresse Gemini Round-2 Review G-R2-1/2/3 (PR #106) ([968b693](968b693)), closes [#106](#106) [#102](#102) [#106](#106) * docs(plan): adresse Gemini-Review G1+G2+G3 (PR #106) ([ad3aec5](ad3aec5)), closes [#106](#106) [#102](#102) [#106](#106) * docs(plan): Phase 1k.2 Multi-Label-Batch implementation plan (#102) ([8917d0e](8917d0e)), closes [#102](#102) [#100](#100) [#102](#102) * docs(research): PT-P750W Layout-Diagnose Befund (Phase 1i A) ([06e8fa9](06e8fa9)) * docs(runbook): UUID-Continuity-Check Phase 1i (MA-2-Fix) ([0ef3861](0ef3861)) * docs(spec): Phase 1k.2 Multi-Label-Batch via ptouch.print_multi (#102) ([96e53c9](96e53c9)), closes [#102](#102) [#101](#101) [#102](#102) [#103](#103) [#102](#102) * docs(spec+plan): adresse Copilot-Review C1-C9 + Privacy-Fixes (PR #106) ([4f56a2a](4f56a2a)), closes [#106](#106) [#102](#102) [#106](#106) * Merge pull request #99 from strausmann/fix/phase1i-ql-preflight-check ([8df1be3](8df1be3)), closes [#99](#99) [skip ci]
Live-Bug aus Phase 1i Smoke-Test
Smoke-Test auf hhdocker03 (QL-820NWB Test-Print über Hangar Admin-Print):
Root Cause
PrintService(app/services/print_service.py:90) ruftawait self._backend.preflight_check()vor jedem Job. Protocol-Methode wurde in Phase 1i Task 8 (BrotherQLBackend) übersehen.Fix
BrotherQLBackend.preflight_check()analog zu PTouchBackend (SNMP via Printer MIB — funktioniert auf QL-820NWB identisch). RaisesPrinterOfflineError/TapeEmptyError/PrinterCoverOpenErrorwie PT.Tests
3 neue Tests: success, tape_empty, snmp_offline.
Refs Phase 1i.