Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix node discovery when compose-node-name is disabled #149

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

simu
Copy link
Member

@simu simu commented Jan 15, 2025

Python reclass will still discover nested nodes if compose-node-name is disabled, but will strip the prefix path from the resulting node names (and the parts metadata).

This commit adjusts reclass-rs to behave identically to Python reclass for inventories which contain nodes in subfolders of inventory/nodes when compose-node-name isn't enabled.

Fixes #148

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 bug Something isn't working label Jan 15, 2025
@simu simu force-pushed the fix/nested-nodes-no-compose branch from 556a9b6 to d064e20 Compare January 15, 2025 17:14
@simu simu requested a review from a team January 15, 2025 17:15

This comment was marked as outdated.

This comment was marked as outdated.

Copy link

@bastjan bastjan left a comment

Choose a reason for hiding this comment

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

I lack some context to understand the code fully but the tests make sense.

This comment was marked as outdated.

@simu simu force-pushed the fix/nested-nodes-no-compose branch from a098c76 to 22ae757 Compare January 15, 2025 20:34

This comment was marked as outdated.

@simu simu force-pushed the fix/nested-nodes-no-compose branch from 4c66715 to 89a582a Compare January 16, 2025 08:46

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

@simu simu force-pushed the fix/nested-nodes-no-compose branch from a9f87e1 to a804388 Compare January 16, 2025 09:06

This comment was marked as outdated.

@simu simu force-pushed the fix/nested-nodes-no-compose branch from a804388 to 1c5efb6 Compare January 16, 2025 09:10

This comment was marked as outdated.

@simu simu mentioned this pull request Jan 16, 2025
4 tasks
simu added 2 commits January 16, 2025 10:23
Python reclass will still discover nested nodes if compose-node-name is
disabled, but will strip the prefix path from the resulting node names
(and the `parts` metadata).

This commit adjusts reclass-rs to behave identically to Python reclass
for inventories which contain nodes in subfolders of `inventory/nodes`
when compose-node-name isn't enabled.

Fixes #148
@simu simu force-pushed the fix/nested-nodes-no-compose branch from 1c5efb6 to 4e7ccc8 Compare January 16, 2025 09:23
Copy link

Benchmark for 3bffcf3

Click to view benchmark
Test Base PR %
Reclass::inventory() multi-threaded 1735.4±127.99µs 1712.7±73.91µs -1.31%
Reclass::inventory() single-threaded 3.6±0.07ms 3.6±0.04ms 0.00%

@simu simu merged commit 201e465 into main Jan 16, 2025
22 checks passed
@simu simu deleted the fix/nested-nodes-no-compose branch January 16, 2025 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reclass-rs doesn't work with latest kapitan (v0.34.4) version
2 participants