-
Notifications
You must be signed in to change notification settings - Fork 0
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
Encrypted databags update #1
Encrypted databags update #1
Conversation
Hey Francesco (@fpedrini), Ha, I'm happy that my code have been useful for you! I promise to do some cleaning on my fork: make it up to date with the upstream's master, then try to rebase my code. Eventually we should be able to put your changes on top of all of it and we end up with a clean pull request to the upstream. Although - I know it's still useful to refresh this branch, so more people can use that. But I don't expect the changes to be merged anyway to the upstream repo, because it wasn't updated for 2 years as for now and I'm not sure if the author (@coderanger) still maintains it. Stay tuned! Best, |
Hey @fpedrini , I merged your code in a clunky way, but it seems to work fine. Also, I cleaned up my branch, kept all the code on kamilbednarz:feature/encrypted-data-bags and I'll try to submit another PR to the upstream. Thank you! |
Thank you very much! |
Hello @kamilbednarz.
I'm opening this PR to your repository as you're the original author for the encrypted databag patches you submitted back in 2014 to the upstream repo in coderanger#27.
I've recently had the need to work with pychef and encrypted databags and your code was immensely helpful (and btw, thank you!), but I had to work a bit around a few issues (mostly around Python 3 compat).
If you're interested, it would be nice to get the changes merged to your fork and the PR to the upstream repo updated, it might increase the chances of getting it merged.
As you can see this PR can't be merged directly with your master branch (as i targeted the upstream HEAD) but it should be possible to work around the issue (the easiest thing, i think, would be to revert the commit on your master branch, update from upstream and then merge this PR, which will restore your commit+the changes needed to apply cleanly against the new HEAD).
Let me know if you're not interested, and I'll open a new PR directly to master (but the attribution in the commits will still be yours).
Also, compared to your original code, I added a new parameter to the
ChefApi
object that allows to pass the encryption key as a string instead of having it loaded from a file. In case bothsecret_file
andsecret_key
are passed, the latter takes precedence.Thanks,
Francesco