Skip to content

fix(brother-ql): doppelter 'preflight SNMP failed' Prefix vermieden#114

Merged
strausmann merged 1 commit into
mainfrom
fix/issue-105-snmp-duplicate-prefix
Jun 9, 2026
Merged

fix(brother-ql): doppelter 'preflight SNMP failed' Prefix vermieden#114
strausmann merged 1 commit into
mainfrom
fix/issue-105-snmp-duplicate-prefix

Conversation

@strausmann

Copy link
Copy Markdown
Owner

Summary

  • Behebt doppelten preflight SNMP failed Prefix in PrinterOfflineError Messages
  • query_preflight() in snmp_helper.py wirft bereits SnmpQueryError("preflight SNMP failed: …") — BrotherQLBackend hat denselben Prefix nochmal hinzugefügt
  • Fix prüft ob Prefix bereits vorhanden, bevor er vorangestellt wird

Closes #105

Test plan

  • Neuer Test test_preflight_check_no_duplicate_prefix_when_snmp_error_already_has_prefix schlägt vor dem Fix fehl (TDD Red → bestätigt Bug)
  • Test grün nach Fix (TDD Green)
  • Bestehende Tests test_preflight_check_raises_offline_on_snmp_error und alle anderen weiterhin grün
  • Alle 733 Unit-Tests grün

🤖 Generated with Claude Code

…Issue #105)

query_preflight() in snmp_helper.py erzeugt SnmpQueryError-Messages die
bereits mit "preflight SNMP failed: " beginnen. BrotherQLBackend hat
denselben Prefix nochmal vorangestellt, was zu doppelten Error-Messages
führte ("preflight SNMP failed: preflight SNMP failed: …").

Fix: prüft ob der Prefix bereits vorhanden ist, bevor er hinzugefügt wird.

Refs #105

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 9, 2026 22:37
@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!

Diese Pull Request behebt ein Problem, bei dem Fehlermeldungen des BrotherQLBackend redundant formatiert wurden. Durch die Überprüfung des bestehenden Fehler-Strings wird sichergestellt, dass das Präfix 'preflight SNMP failed' nur einmalig in der finalen Exception erscheint, was die Lesbarkeit der Logs verbessert.

Highlights

  • Fehlerbehebung bei Fehlermeldungen: Verhindert die doppelte Präfixierung von 'preflight SNMP failed' in PrinterOfflineError-Meldungen.
  • Implementierung einer Prüfung: Der Code prüft nun, ob der Fehler-String bereits das Präfix enthält, bevor es erneut hinzugefügt wird.
  • Erweiterte Testabdeckung: Ein neuer Unit-Test wurde hinzugefügt, um sicherzustellen, dass das Präfix bei SnmpQueryError nicht dupliziert wird.
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.

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

Copy link
Copy Markdown

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 addresses Issue #105 by preventing duplicate 'preflight SNMP failed' prefixes in PrinterOfflineError messages when handling SnmpQueryError in the Brother QL backend, and adds a corresponding unit test. The review feedback suggests a more robust prefix check (checking for 'preflight SNMP' instead of 'preflight SNMP failed:') to handle other potential error message variations and avoid duplicate prefixes.

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.

Comment on lines +178 to +181
msg = str(exc)
if not msg.startswith("preflight SNMP failed:"):
msg = f"preflight SNMP failed: {msg}"
raise PrinterOfflineError(msg) from exc

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Wenn query_preflight() einen SnmpQueryError mit einer Nachricht wie "preflight SNMP returned error: ..." wirft, startet diese nicht mit "preflight SNMP failed:". Dadurch wird die Fehlermeldung zu "preflight SNMP failed: preflight SNMP returned error: ..." formatiert, was ein doppeltes "preflight SNMP"-Präfix erzeugt.

Eine robustere Prüfung wäre es, auf "preflight SNMP" zu prüfen, um jegliche doppelte Präfixe für alle von query_preflight() geworfenen Fehler zu vermeiden.

Suggested change
msg = str(exc)
if not msg.startswith("preflight SNMP failed:"):
msg = f"preflight SNMP failed: {msg}"
raise PrinterOfflineError(msg) from exc
msg = str(exc)
if not msg.startswith("preflight SNMP"):
msg = f"preflight SNMP failed: {msg}"
raise PrinterOfflineError(msg) from exc

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Diese Änderung behebt ein kosmetisches Problem in der Fehlerbehandlung des BrotherQLBackend: Wenn query_preflight() bereits eine SnmpQueryError-Message mit dem Prefix preflight SNMP failed: liefert, soll BrotherQLBackend.preflight_check() diesen Prefix nicht nochmals voranstellen (verhindert Meldungen wie preflight SNMP failed: preflight SNMP failed: …). Das passt zur in Issue #105 beschriebenen Phase-1j-Qualitätsbereinigung.

Changes:

  • Entdoppelt den preflight SNMP failed: Prefix in BrotherQLBackend.preflight_check() durch eine startswith(...)-Prüfung.
  • Fügt einen Unit-Test hinzu, der den Doppel-Prefix-Fall reproduziert und sicherstellt, dass die Message den Prefix nur einmal enthält.

Reviewed changes

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

File Description
backend/app/printer_backends/brother_ql_backend.py Verhindert doppeltes Prefixing der PrinterOfflineError-Message bei bereits präfixierten SnmpQueryErrors.
backend/tests/unit/printer_backends/test_brother_ql_backend.py Ergänzt einen Regressionstest für den bereits-präfixierten SNMP-Fehlerfall (Issue #105).

@codecov

codecov Bot commented Jun 9, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.21%. Comparing base (ddcf114) to head (f5edd6d).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #114      +/-   ##
==========================================
+ Coverage   89.18%   89.21%   +0.03%     
==========================================
  Files          91       91              
  Lines        4243     4246       +3     
  Branches      365      366       +1     
==========================================
+ Hits         3784     3788       +4     
  Misses        360      360              
+ Partials       99       98       -1     
Components Coverage Δ
Printer Backends (transport) 86.87% <100.00%> (+0.11%) ⬆️
Printer Models (drivers) 88.20% <ø> (ø)
Services 91.17% <ø> (+0.07%) ⬆️
REST API 84.97% <ø> (ø)
Pydantic Schemas 100.00% <ø> (ø)
Integration Plugins 100.00% <ø> (ø)
Files with missing lines Coverage Δ
backend/app/printer_backends/brother_ql_backend.py 88.00% <100.00%> (+0.76%) ⬆️

... and 1 file with indirect coverage changes

Flag Coverage Δ
backend 89.21% <100.00%> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba7136d...f5edd6d. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@strausmann
strausmann merged commit e459fd7 into main Jun 9, 2026
20 checks passed
@strausmann
strausmann deleted the fix/issue-105-snmp-duplicate-prefix branch June 9, 2026 22:40
github-actions Bot pushed a commit that referenced this pull request Jun 10, 2026
## <small>0.10.1 (2026-06-10)</small>

* fix(brother-ql): doppelter 'preflight SNMP failed' Prefix vermieden (Issue #105) (#114) ([e459fd7](e459fd7)), closes [#105](#105) [#114](#114) [#105](#105)
* fix(examples): aktualisiere compose.pangolin.yml und .env.example auf aktuelle Konfiguration (#113) ([a16643e](a16643e)), closes [#113](#113) [#73](#73)
* chore(deps): bump the go-minor-and-patch group across 1 directory with 3 updates (#112) ([3939741](3939741)), closes [#112](#112)
* docs: Phase 1k.1 Layout-Engine + TapeGeometry + ContentTypes (Spec + Plan) (#108) ([a5425a5](a5425a5)), closes [#108](#108) [#103](#103) [#108](#108) [#103](#103) [#103](#103) [#103](#103) [#103](#103) [#103](#103) [#103](#103) [#103](#103) [#103](#103) [#103](#103)
* ci(deps): bump codecov/codecov-action in the actions-all group (#111) ([2a9454b](2a9454b)), closes [#111](#111)

[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.

Phase 1j: BrotherQLBackend.query_status duplicate SNMP prefix in error message

2 participants