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

Call ssl.create_default_context conditionally, depending on the pytho… #60

Merged
merged 3 commits into from
Jan 17, 2018
Merged

Conversation

afester
Copy link
Contributor

@afester afester commented Jan 15, 2018

#59 added a call to ssl.create_default_context which is only available starting with Python 2.7.9. At least FreeCad 0.16 build 6712 on Windows 10 still uses Python 2.7.8.
This pull request encapsulates the call to urllib2.connect() and only calls ssl.create_default_context when the Python version is 2.7.9 or later.

@OctagonHex
Copy link

I think there is an error, the openUrl helper function:
it reads in the second branch:
return urllib2.urlopen(url)
instead of
return urllib2.urlopen(url, , context=ctx)
otherwise the context is never used.

@elanlb
Copy link
Contributor

elanlb commented Jan 15, 2018

Yes, I agree with @OctagonHex , you defined a variable for the context but you didn't actually use it. I added this context thing to every class regardless of the Python version in this commit, so you can look at that and see an example of the implementation. The urlopen function should be called like this: urllib2.urlopen("https://github.com/FreeCAD/FreeCAD-addons", context=ctx)

(This is also an improvement to issue #11 )

@afester
Copy link
Contributor Author

afester commented Jan 15, 2018

Absolutely correct, my bad - I was probably too much in a hurry to get the code updated for the #57 PR ... Will fix that and update the PR

@yorikvanhavre yorikvanhavre merged commit 35cecf4 into FreeCAD:master Jan 17, 2018
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.

None yet

4 participants