Skip to content

Implement ignore_overwritten_missing_references kapicorp-reclass config option#185

Merged
simu merged 10 commits intomainfrom
feat/ignore-missing-overwritten-references
Nov 25, 2025
Merged

Implement ignore_overwritten_missing_references kapicorp-reclass config option#185
simu merged 10 commits intomainfrom
feat/ignore-missing-overwritten-references

Conversation

@simu
Copy link
Copy Markdown
Member

@simu simu commented Sep 25, 2025

This PR changes reclass-rs to allow missing overwritten references in scalar target values by default to match kapicorp-reclass behavior.

Additionally, this PR implements the kapicorp-reclass config option ignore_overwritten_missing_reference which allows users to tune the behavior (and restore the previous reclass-rs behavior of aborting on missing references in scalar target values).

Fixes #184

TODO

  • Implement config option to tune this behavior
  • Add test cases for this behavior -- simple case works, but I'd like to test some more complex scenarios
  • Decide whether to default to stay compatible with kapicorp-reclass or whether to default to making missing overwritten references an error.
  • Check if we can have a mode where we collect all render errors and return them at the end
  • Update README

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.

@simu simu added the enhancement New feature or request label Sep 25, 2025
@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@simu simu force-pushed the feat/ignore-missing-overwritten-references branch from 7a70f9a to aae4c56 Compare October 3, 2025 09:06
@github-actions

This comment was marked as outdated.

@simu simu changed the title Add support for allowing missing overwritten references in scalar target values Add support for ignore_overwritten_missing_references kapicorp-reclass config option Oct 3, 2025
@simu simu changed the title Add support for ignore_overwritten_missing_references kapicorp-reclass config option Implement ignore_overwritten_missing_references kapicorp-reclass config option Oct 3, 2025
@simu simu force-pushed the feat/ignore-missing-overwritten-references branch from aae4c56 to a4819e4 Compare November 24, 2025 18:55
simu added 4 commits November 24, 2025 19:57
…e()`

This addresses the `clippy::implicit_clone` lint.
The helper returns true for all Value variants other than Mapping,
Sequence and ValueList.
We default the option to `true` to retain the kapicorp-reclass default
behavior. In contrast to kapicorp-reclass, we can't currently collect
all resolution errors but instead hard-fail on the first non-ignored
error we encounter.

Technically this is a breaking change for existing reclass-rs
inventories.

The implementation introduces a new Value variant `ResolveError` which
is used by the Value implementation to decide which resolve errors are
always errors and which can be downgraded to warnings when Reclass
option `ignore_overwritten_missing_references` is set to `true`.

Fixes #184
@github-actions

This comment was marked as outdated.

@simu simu force-pushed the feat/ignore-missing-overwritten-references branch from a4819e4 to 0c7b42c Compare November 24, 2025 19:02
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 24, 2025

Benchmark for d6be1b2 (Without new node in inventory)

Click to view benchmark
Test Base PR %
Reclass::inventory() multi-threaded 1450.1±39.78µs 1463.9±113.91µs +0.95%
Reclass::inventory() single-threaded 2.6±0.04ms 2.6±0.12ms 0.00%

@simu simu force-pushed the feat/ignore-missing-overwritten-references branch 2 times, most recently from 6427844 to 6cc3baa Compare November 24, 2025 19:08
@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@simu
Copy link
Copy Markdown
Member Author

simu commented Nov 25, 2025

Performance overhead for a real Commodore inventory (APPUiO Lab) is negligible:

# kapitan inventory --inventory-backend=reclass-rs --inventory-path /path/to/inventory -v >/dev/null
[ ... snipped ... ]
Inventory rendering with reclass-rs took 0:00:03.214301
[ ... snipped ... ]
# ./inv.py /path/to/inventory > /dev/null
inventory took 0:00:03.216774
[ ... snipped ... ]

@simu
Copy link
Copy Markdown
Member Author

simu commented Nov 25, 2025

Check if we can have a mode where we collect all render errors and return them at the end

We could do this, but we'd need to refactor a bunch of state tracking to make it actually work properly, so we leave it for now.

@simu simu marked this pull request as ready for review November 25, 2025 12:45
@simu
Copy link
Copy Markdown
Member Author

simu commented Nov 25, 2025

PyO3 deprecation warnings are addressed in #191

@simu simu requested a review from a team November 25, 2025 12:46
@simu simu force-pushed the feat/ignore-missing-overwritten-references branch from 686fd2d to ded03b2 Compare November 25, 2025 12:48
@github-actions

This comment was marked as outdated.

@simu simu force-pushed the feat/ignore-missing-overwritten-references branch from ded03b2 to 974108a Compare November 25, 2025 13:04
@github-actions

This comment was marked as outdated.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 25, 2025

Benchmark for 514013a (With new node in bench inventory)

Same implementation as benchmark results in #185 (comment) but with a new inventory node n26 which has a bunch of overwritten missing references in the classes it includes.

Click to view benchmark
Test Base PR %
Reclass::inventory() multi-threaded 1740.0±136.85µs 2.3±0.19ms +32.18%
Reclass::inventory() single-threaded 3.6±0.10ms 4.6±0.04ms +27.78%

simu added 6 commits November 25, 2025 16:55
…ing override mapping keys

We also add some diagnostic error messages when dropping unrendered
mapping values.
Users can opt in to those warnings by setting `verbose_warnings: true`
in their inventory's `reclass-config.yml`.
@simu simu force-pushed the feat/ignore-missing-overwritten-references branch from 5e7bb80 to de64609 Compare November 25, 2025 15:56
@github-actions
Copy link
Copy Markdown

Benchmark for 38f7e84

Click to view benchmark
Test Base PR %
Reclass::inventory() multi-threaded 1726.5±135.52µs 2.3±0.18ms +33.22%
Reclass::inventory() single-threaded 3.5±0.03ms 4.5±0.05ms +28.57%

@simu
Copy link
Copy Markdown
Member Author

simu commented Nov 25, 2025

macOS 13 EOL handled in #193

@simu simu merged commit 01a46e3 into main Nov 25, 2025
21 of 22 checks passed
@simu simu deleted the feat/ignore-missing-overwritten-references branch November 25, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

reclass-rs doesn't allow overwritten missing references in scalar values

2 participants