Skip to content

Serialize YAML null as string "null" in nested references#207

Open
simu wants to merge 2 commits intomainfrom
fix/ref-default-value-null
Open

Serialize YAML null as string "null" in nested references#207
simu wants to merge 2 commits intomainfrom
fix/ref-default-value-null

Conversation

@simu
Copy link
Copy Markdown
Member

@simu simu commented Apr 29, 2026

Until now, we serialized Value::Null to "None" to preserve compatibility with Python's str() in nested reference lookups (since kapicorp-reclass just uses str() to serialize nested references).

However, this causes issues when using a nested reference that resolves to a YAML null value as a default value for a reclass reference: until now such a reference would result in a YAML string "None" when the resolution falls back on the default value (see parameter nullref in tests/inventory-reference-default-values/nodes/n12.yml for an example reference that exhibits this behavior).

This PR changes the default serialization of Value::Null while resolving nested references to "null". This value is preserved as a YAML null when a resolved nested reference is parsed again because it's used as a reference default value.

Note that this change breaks existing nested references for inventories that expect YAML null to be serialized as None. However, to accommodate users of such inventories, the PR also introduces a new compatibility flag NestedReferenceNullAsNone which changes reclass-rs's serialization of YAML null values in nested references to emit string "None" to preserve kapicorp-reclass compatibility. Unless your inventory depends on this behavior we strongly recommend not using the compatibility mode.

Follow-up / fix for #194

Marked as breaking because the new default behavior (without the compatibility flag) is a breaking change for existing inventories that expect nested references to resolve YAML null values as string "None".

Checklist

  • The PR has a meaningful title. The title will be used to auto generate the changelog
  • PR contains a single logical change (to build a better changelog).
  • Update the documentation.
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, enhancement, documentation, change, breaking, dependency, internal
    as they show up in the changelog
  • Link this PR to related PRs or issues.

Until now, a reference with a default value which is a reference that
resolves to a YAML `null` value would result in a YAML string `"None"`
when the resolution falls back on the default value.

The cause for this is that we serialized `Value::Null` to `"None"` until
now to preserve compatibility with Python's `str()` for nested reference
lookups (since Python Reclass just uses `str()` to serialize nested
references).

This commit changes the serialization of `Value::Null` to `"null"` which
is preserved as a YAML `null` when a reference default value is defined
as a nested reference that resolves to YAML `null`.

Conversely, any existing nested references that expect YAML `null` to be
serialized as `None` will break with this change.
@simu simu requested a review from a team April 29, 2026 12:51
@simu simu added the breaking label Apr 29, 2026
@simu simu changed the title Serialize YAML null as string "null" in nested references Serialize YAML null as string "null" in nested references Apr 29, 2026
@github-actions

This comment was marked as outdated.

@simu simu force-pushed the fix/ref-default-value-null branch from e91d02f to adb73c4 Compare April 29, 2026 12:57
@github-actions

This comment was marked as outdated.

This compat flag changes reclass-rs's serialization of YAML `null` values in
nested references to emit string "None" instead of the new default of
string "null". Unless your inventory depends on this behavior we
strongly recommend not using the compatibility mode.
@simu simu force-pushed the fix/ref-default-value-null branch from adb73c4 to 0b4365a Compare April 29, 2026 13:02
@github-actions
Copy link
Copy Markdown

Benchmark for bb3ff00

Click to view benchmark
Test Base PR %
Reclass::inventory() multi-threaded 2.1±0.16ms 2.1±0.16ms 0.00%
Reclass::inventory() single-threaded 4.5±0.04ms 4.5±0.02ms 0.00%

Copy link
Copy Markdown
Member

@haasad haasad left a comment

Choose a reason for hiding this comment

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

LGTM (although my understanding of this codebase and rust in general is limited)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants