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

Ensure stable key order when processing hashes #799

Closed
wants to merge 1 commit into from

Conversation

luitzifa
Copy link

@luitzifa luitzifa commented Feb 6, 2025

We encountered a problem with changing config files because the data was dynamically generated.

Even though it might just be possible to create a stable insertion order in other places, this should really not be necessary. We want to generate stable icinga config files without having to jump though any extra hoops.

@SimonHoenscheid
Copy link
Member

The tests should work before we merge this. The change itself looks good and makes sense

@lbetz
Copy link
Contributor

lbetz commented Feb 6, 2025

First, please let us known what puppet version and which version of ruby you're using? The behavior you describe sounds like ruby 1.8.7.

Even though it might just be possible to create a stable
insertion order in other places, this should really
not be necessary. We want to generate stable icinga
config files without having to jump though any extra hoops.
@luitzifa
Copy link
Author

luitzifa commented Feb 6, 2025

I'm using

# puppetserver ruby --version
jruby 9.3.14.0 (2.6.8) 2024-02-20 0db23ddd14 OpenJDK 64-Bit Server VM 17.0.14+7-Debian-1deb12u1 on 17.0.14+7-Debian-1deb12u1 +jit [x86_64-linux]
# puppetserver --version
puppetserver version: 7.17.3

on Debian Bookworm provided by puppetlabs.

The problem arises if the order of the keys in the hash is not stable. This affects us because the hashes originate from puppetdb queries.

@lbetz
Copy link
Contributor

lbetz commented Feb 6, 2025

What kind of queries? By the module itself?

@lbetz
Copy link
Contributor

lbetz commented Feb 6, 2025

The order is not so easy to change, there are users for whom this is important.

@luitzifa
Copy link
Author

luitzifa commented Feb 7, 2025

The order is not so easy to change, there are users for whom this is important.

I didn't thought about that.

The puppetdb queries are made by our code. It is certainly possible to determine the order on our site in advance.

@lbetz
Copy link
Contributor

lbetz commented Feb 7, 2025

"The puppetdb queries are made by our code. It is certainly possible to determine the order on our site in advance."
Would be the easier and shorter way if it's possible, I think.

@luitzifa
Copy link
Author

luitzifa commented Feb 7, 2025

We'll do that. It's fine by me, if you reject this PR.

@lbetz lbetz added the wontfix label Feb 7, 2025
@lbetz lbetz closed this Feb 7, 2025
@luitzifa luitzifa deleted the sort_attrs_hash branch February 10, 2025 07:39
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.

3 participants