#493 fix event rule handling for NetBox 4.5

[arthanson]

Fixes: #493

process_event_rules function signature got changed in NetBox 4.5.2, this uses version check to make it forwards and backwards compatible. The other method is via using inspect, but thought this was lighter weight.

[arthanson]

@claude /review

[claude]

Claude finished @arthanson's task in 1m 23s —— View job

---

Code Review: #493 fix event rule handling for NetBox 4.5

• Gather context
• Read changed files
• Analyze changes
• Post review

---

Overview

This PR fixes a breaking change in process_event_rules introduced in NetBox 4.5.2, where event_type, data, and username were consolidated into a single event dict parameter. The fix adds a runtime version check to call the correct signature depending on the installed NetBox version, preserving backward compatibility with 4.4.x–4.5.1.

The original commit (aa8402b) simply updated to the new signature (breaking older versions), then ae68593 added the version check for backward compatibility.

---

Assessment

Correctness ✅

The version boundary (4, 5, 2) matches the described API change, and the new event dict structure {'event_type': event_type, 'data': data, 'username': username} aligns with the 4.5.2 signature.

Issues

1. Fragile version parsing — settings.RELEASE.version.split('.')[:3] with int(x) will raise ValueError on pre-release NetBox builds (e.g. 4.5.2.dev0, 4.5.2rc1, 4.5.2b1). While production releases use clean X.Y.Z strings, dev/CI environments may not.

   signal_receivers.py:240 — consider using a try/except or stripping non-numeric suffixes. Fix this →

2. Version re-parsed on every call — nb_version is computed on each invocation of handle_branch_event. This is negligible for event handling (low frequency), but it could be a module-level constant or cached. Minor nit.

3. inspect is arguably more robust — The PR description mentions inspect was rejected as "heavier." In practice, inspect.signature(process_event_rules) is called once and would survive any future API changes without needing to update a hardcoded version boundary. The version-sniffing approach requires knowing exactly which version introduced the change. However, the chosen approach is acceptable for this known, specific change.

4. No tests — Expected given the testing constraints (requires running NetBox), but the new code path (the >= 4.5.2 branch) is not covered. If a test fixture for handle_branch_event exists in tests/test_events.py, it's worth verifying it still passes against 4.5.2.

---

Minor Style

The import from django.conf import settings was correctly added at line 6, following the project's import ordering (stdlib → Django → third-party → local).

---

Summary

The fix is functionally correct and the version-based dispatch approach is reasonable for this project's supported version range (4.4.1–4.6.99). The primary concern worth addressing before merge is the robustness of version string parsing for non-release builds. Everything else is minor.