-
Notifications
You must be signed in to change notification settings - Fork 248
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
Use winkerberos on windows #504
base: master
Are you sure you want to change the base?
Conversation
This switches impyla to depend on `winkerberos` on windows rather than `kerberos`. The `winkerberos` package provides pre-built wheels for `windows` with the same python-facing API.
a078a86
to
4bd7592
Compare
Thanks for working on this! I have concerns about testing though: I don't know whether anyone tests Impyla with Windows + Kerberos, so it seems possible to break some use cases without noticing it. All the test environments I know about are Linux based. |
There are not, and I also have no way to test this. I've had similar code in place for years in another kerberos-related project though with no complaints, but this may just mean that no one is using it on windows :). This PR was quick to work up, no pressure at all to merge it. Apologies that I can't be more help here ensuring this is correct. |
I started asking around about Impyla + Windows + Kerberos inside Cloudera. I was thinking about an alternative in case switching to winkerberos from kerberos would be problematic for some people:
This way an easy installation could be provided on Windows without the chance of breaking any existing use cases. |
I didn't get any feedback yet about this setup. @jcrist Is it ok for you to proceed this way? |
Based on another PR: cloudera#504
This switches impyla to depend on
winkerberos
on windows rather thankerberos
. Thewinkerberos
package provides pre-built wheels forwindows
with the same python-facing API.Fixes #466.