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
Foreman facts uploading correctly merges facts #1155
Conversation
Makes the callback work much better! 👍 (I don't have merge permissions here) |
@ares uh, you do! @adamruzicka thanks! Could you please add a changelog snippet to https://github.com/theforeman/foreman-ansible-modules/tree/develop/changelogs/fragments? |
Also, this seems to have more changes than just the new merging? ;) |
There are several refactorings that were made in the original repo I think. |
@adamruzicka please let me know when you add a changelog, if I really have merge permissions, I'm happy to merge afterwards :-) |
The callback plugin collects facts during the run, merges them correctly and uploads them once at the end. Adapted from ansible/ansible#51546
Updated and the tests are 🍏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @adamruzicka, merging!
@ares @adamruzicka what in this PR requires requests 2.14? That's not available on EL7, making the callback less useful on one of our main platforms |
@evgeni Built in json encoding ansible/ansible@11fab2d |
So post(json=…)? Interesting. |
I'd argue it's there since 2.4.2 (and that's why apypie depends on >= 2.4.2): psf/requests@8f17741 |
cc @ekohl could the 14 be a typo? |
And 2.14 is from 2017: https://github.com/psf/requests/releases/tag/v2.14.0 So the explanation in that commit is wrong |
It does look like I made a mistake here, but at least I documented why it was needed. And the 2014 was correct :) |
Fixed in #1205 |
Adapted from ansible/ansible#51546