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

Encrypted databags update #1

Conversation

fpedrini
Copy link

@fpedrini fpedrini commented Sep 4, 2018

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 both secret_file and secret_key are passed, the latter takes precedence.

Thanks,
Francesco

kamilbednarz and others added 30 commits September 4, 2018 16:41
@kamilbednarz
Copy link
Owner

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,
Kamil

@kamilbednarz kamilbednarz changed the base branch from master to feature/encrypted-data-bags September 5, 2018 21:37
@kamilbednarz kamilbednarz merged commit f6a6773 into kamilbednarz:feature/encrypted-data-bags Sep 5, 2018
@kamilbednarz
Copy link
Owner

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!

@fpedrini
Copy link
Author

fpedrini commented Sep 6, 2018

Thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants