fix: preserve explicit empty values and null refs in update changesets#162
Open
mfiedorowicz wants to merge 2 commits intodevelopfrom
Open
fix: preserve explicit empty values and null refs in update changesets#162mfiedorowicz wants to merge 2 commits intodevelopfrom
mfiedorowicz wants to merge 2 commits intodevelopfrom
Conversation
… update changesets
Previously, clean_diff_data() stripped empty strings, None values, and
empty dicts from changeset data unconditionally. This made it impossible
for users to clear optional fields (e.g. reset description to "" or
remove a tenant assignment) via diode ingestion — the "clear" signal
was indistinguishable from "field not provided".
Differ: for UPDATE changesets, stop stripping empty values from the data
dict so that explicitly-provided empty strings and None values survive
into the changeset. CREATE changesets retain the existing behavior.
Transformer: handle null and empty-dict ({}) values for reference fields.
An empty dict comes from protojson when the SDK sets a message field to
an empty message (e.g. Tenant{}) to signal "clear this field". Both null
and {} are now treated as "clear this FK" for regular FKs, generic FKs,
and circular reference fields.
mfiedorowicz
requested review from
MicahParks,
jajeffries,
leoparente,
ltucker and
marc-barry
as code owners
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
jajeffries
approved these changes
Apr 15, 2026
ldrozdz93
approved these changes
Apr 15, 2026
ldrozdz93
left a comment
ldrozdz93
left a comment
There was a problem hiding this comment.
LGTM
Just to note, IIUC this does not cover clearing custom_fields or tags. I can easily imagine customers wanting to keep their netbox device or networks tags in sync with their infra, including the removal of old tags.
|
For the record: I run that locally and it looks fine. Interface assigned to an IP was properly removed from that IP. |
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
3 participants
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.
Summary
Related to netboxlabs/diode#421
"") andNonevalues from changesetdata. This allows users to explicitly clear optional fields (e.g. resetdescriptionto""or remove atenantassignment) via diode ingestion. CREATE changesets retain existing behavior (strip empty values to avoid noise).nulland{}(empty dict) values for reference fields. An empty dict comes fromprotojsonwhen the SDK sets a message field to an empty message (e.g.Tenant{}) to signal "clear this field". Both are treated as "clear this FK" for regular FKs, generic FKs (e.g.assigned_object), and circular reference fields.Background
Previously,
clean_diff_data()stripped empty strings,None, empty lists, and empty dicts unconditionally. This made three scenarios indistinguishable:"description": ""data"tenant": nullor"tenant": {}None, preservedA companion test in
diode-server(branchtest/protojson-field-presence-behavior) confirms thatprotoToJSON()already emits the correct signals — no Go server changes needed:optional stringnot set → omitted; set to""→"description": ""optional Tenantnil → omitted; set toTenant{}→"tenant": {}oneofnot set → all variants omitted; set toInterface{}→"assigned_object_interface": {}Test plan