fix: netbox-branching 1.1.1 compat + endpoint identity clamping (2.4.5)#46
Merged
Merged
Conversation
netbox-branching 1.1.1 removed SquashMergeStrategy._split_bidirectional_cycles
(and _has_fk_to); the bulk merge called it unconditionally, so a fresh install
resolving the unbounded >=1.1.0 dependency to 1.1.1 died mid-sync with an
AttributeError. The bidirectional-cycle splitter is now vendored (faithful
mirror of the 1.1.0 idiom including GenericForeignKey cycle detection); the
framework helper is preferred when present so 1.1.0 behavior is unchanged, and
the dependency is bounded to >=1.1.0,<1.2.0 so future internal churn fails at
install time rather than mid-merge. _log_cycle_details is guarded the same way
and tests tolerate either branching version.
Endpoint rows carry raw SNMP sysDescr as device_type, which exceeds NetBox's
100-char DeviceType model/slug limits (observed 251 chars -> 18 per-row
"Ensure this value has at most 100 characters" rejects) and can slugify to ""
("At least one coalesce lookup must be provided"). Device-row taxonomy fields
are now clamped to 100 chars with deterministic slug recomputation and
fallbacks, applied on every fetch exit including fetch-artifact cache hits so
resumed runs replay clean rows. Device names are never modified.
Full suite (1001) + lint + harness green.
Maintainer decision: NQE is the source of truth — the plugin must not normalize or mutate query rows. Replace the Python _sanitize_device_rows mutation with query-side clamping in both endpoint branches: device_type = substring(sysDescr, 0, 100), slug clamped after slugifyNetboxModel (which can EXPAND the string past 100 — live-verified 124 -> 100), and explicit fallbacks for empty slugs/manufacturers. Because the queries changed, operators publish bundled queries after upgrading; the release notes and version tables say so. Live-validated on ADP: both device queries emit max(model)=100 max(slug)=100 with zero empty slugs across 5645 rows / 685 endpoints. Full suite (997) + lint + harness + NQE lint green.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem (field report, design partner on 2.4.4)
type object 'SquashMergeStrategy' has no attribute '_split_bidirectional_cycles'— netbox-branching 1.1.1 (released; our dependency was unbounded>=1.1.0) removed that internal helper (and_has_fk_to); our bulk merge called it unconditionally.device_type— up to 251 chars vs NetBox's 100-char DeviceType model/slug limits, plus one symbol-only sysDescr slugifying to""(At least one coalesce lookup must be provided).Changes
_split_bidirectional_create_cycles+_create_has_fk_to(faithful 1.1.0 mirror incl. GenericForeignKey cycle detection); prefer the framework helper when present (1.1.0 bit-identical); guard_log_cycle_details; tests tolerate either branching version (create=Truepatches, skip-if-absent).netboxlabs-netbox-branching >=1.1.0,<1.2.0— internals are borrowed by design, so future churn should fail at install, not mid-merge._sanitize_device_rowsclamps device_type/manufacturer/platform/role/site + slugs to 100 chars, recomputes the slug from the clamped value (deterministic — no DeviceType churn), falls back tounknownfor degenerate slugs; applied in_apply_device_tag_scopeand on every_fetch_spec_rowsexit so fetch-artifact cache hits (resumed runs) are clean too. Device names never modified.Verification