Merge lp:~ahasenack/txaws/no-local-dir-for-ca-lookup into lp:txaws
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Christopher Armstrong on 2012-10-30 | ||||
Approved revision: | 158 | ||||
Merged at revision: | 151 | ||||
Proposed branch: | lp:~ahasenack/txaws/no-local-dir-for-ca-lookup | ||||
Merge into: | lp:txaws | ||||
Diff against target: | 0 lines | ||||
To merge this branch: | bzr merge lp:~ahasenack/txaws/no-local-dir-for-ca-lookup | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Christopher Armstrong | Approve on 2012-10-30 | ||
Jamu Kakar | 2012-10-29 | Approve on 2012-10-29 | |
Review via email:
|
Description of the change
This branch changes get_ca_certs() so that it does not accidentally include the current directory when looking for *.pem CA files to load.
Andreas Hasenack (ahasenack) wrote : | # |
That docstring is not accurate. What I'm testing is that get_ca_certs() does not include the current directory if TXAWS_CERTS_PATH ends with a ":".
Good point about the variable remaining tainted, will fix, thanks.
Jamu Kakar (jkakar) wrote : | # |
Hehe, that's a good reason to provide clear docstrings with tests.
They can be so easy to misunderstand, especially when you read the
code before the morning coffee has hit your bloodstream. ;b
Andreas Hasenack (ahasenack) wrote : | # |
Can't I just put the os.chdir() that goes back to the original dir in that finally block too?
Andreas Hasenack (ahasenack) wrote : | # |
NM, got some addCleanup() introduction from radix :)
Jamu Kakar (jkakar) wrote : | # |
Yeah, addCleanup is your friend.
Andreas Hasenack (ahasenack) wrote : | # |
Ok, how about this now.
Andreas Hasenack (ahasenack) wrote : | # |
Having fun here with such a simple case :)
v3.0!
- 158. By Andreas Hasenack on 2012-10-30
-
Tidy up
Christopher Armstrong (radix) wrote : | # |
Looks good. Thanks for moving the os.getcwd() call and fixing the other tests. +1
[1]
The test is tainting the TXAWS_CERTS_PATH environment variable. You
could do something like the following. Also, I recommend you add a
simple sentence clearly describing the expected behaviour.
def test_get_ ca_certs_ no_current_ dir(self) : Error} exception is raised if no certificate paths are certs_path = os.environ. get("TXAWS_ CERTS_PATH" )
os.environ[ "TXAWS_ CERTS_PATH" ] = "%s:" % self.no_certs_dir
os.chdir( self.one_ cert_dir)
self.assertRai ses(exception. CertsNotFoundEr ror, ssl.get_ca_certs) certs_path:
os. environ[ "TXAWS_ CERTS_PATH" ] = original_certs_path "TXAWS_ CERTS_PATH" ]
"""
A L{CertsNotFound
available.
"""
original_
try:
finally:
if original_
else:
del os.environ[
Nice work, +1! :)