-
Notifications
You must be signed in to change notification settings - Fork 49
Configure Windows at import time #194
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
Conversation
And that's going to raise a KeyError on Linux.... One moment. |
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.
Some minor changes, but this seems like a good thing to have!
gssapi/_win_config.py
Outdated
@@ -0,0 +1,55 @@ | |||
""" | |||
Using GSSAPI on Windows requires having an installation of Kerberos for Windows | |||
(K4W) available in the user's PATH. This module should be imported before |
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.
MIT seems to call this KfW (or kfw) - shouldn't we use the same terminology here? https://web.mit.edu/kerberos/kfw-4.1/kfw-4.1.html
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.
You would be correct. No idea why I had it in my head as K4W.
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.
Should be all set
gssapi/_win_config.py
Outdated
'MIT', 'Kerberos', 'bin', | ||
) | ||
#: Download location for K4W 64bit MSI | ||
K4W_DL = "http://web.mit.edu/KERBEROS/dist/kfw/4.1/kfw-4.1-amd64.msi" |
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.
I think it might be better to link https://web.mit.edu/kerberos/ so that they can grab the latest version - what do you think?
Also, https please :)
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.
Sure thing. I used the download to make sure they would grab the right one, so I'll just add some more instructions instead.
I copied the link from that page directly, not sure why it is http.
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.
Should be all set
[[email protected]: Squash commits] Merges: #194
As I described in #193, it would probably help out our users to add K4W to the PATH at import time if necessary and yield helpful errors if we can't find it.