-
-
Notifications
You must be signed in to change notification settings - Fork 29.2k
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
Get rid of IOError. Use OSError instead #60919
Comments
New changeset 7d69d04522e3 by Andrew Svetlov in branch 'default': |
Fixed. |
found a typo (removed LoadError) commit 80f2b1273e8c0e1a28fabe537ae9c5acbbcee187
... diff --git a/Lib/http/cookiejar.py b/Lib/http/cookiejar.py ... @@ -1786,7 +1786,7 @@ class FileCookieJar(CookieJar):
self._cookies = {}
try:
self.load(filename, ignore_discard, ignore_expires)
- except (LoadError, IOError):
+ except OSError:
self._cookies = old_state
raise |
It's not a typo.
The code is correct from my perspective. |
Andrew Svetlov wrote:
the comment for the function doesn't tell it |
LoadError was IOError descendant, now OSError is directly specified. |
Zen: I assume that someone changes the LoadError class and goes into the revert function. How he can figure out that the cookies should return to the previous state if LoadError was raised ? Before the change it was explicit and after the change it became implicit. |
Please email python-dev if you think LoadError should be directly specified. |
Source code says: # derives from OSError for backwards-compatibility with Python 2.4.0
class LoadError(OSError): pass In 3.0, LoadError could have been made a direct subclass of Exception. Now it’s too late, but a best practice IMO would still be to write LoadError in all new code. This makes no practical difference, unless test_cookiejar does not check that LoadError is properly raised and caught at the right places. In that case, it may be a little more future-proof to follow py-user’s advice. |
Now that 3.2 is off of maintenance and Idle patches are being applied to both 3.3 and 3.4 (and usually 2.7), I plan to backport the Idle subset of changes to 3.3 so patches are more likely to merge forward without incident. (I have already done one where this change was the only problem.) |
New changeset 2fe64ce5da05 by Terry Jan Reedy in branch '3.3': |
Commit to 3.3 broke at least my FreeBSD buildbot: Also setting +Version: Python 3.3 on this. |
I don't think so. The idle test, test_idle, passed. The patch did not even affect any of the three idle files that it currently tests. Just because a commit triggers a test does not mean that it is the cause of any failure that happens. The log says the failure was in test_subprocess. Traceback (most recent call last):
File "/usr/home/buildbot/python/3.3.koobs-freebsd/build/Lib/test/test_subprocess.py", line 953, in test_wait_timeout
p.wait(timeout=0.0001)
AssertionError: TimeoutExpired not raised
'''
As I interpret the traceback, this did not fail is the re-run. I did not set 3.3, nor reopen, because this patch is primarily part of a new issue, bpo-18151. I cross-referenced this one so that if anyone else thought of backporting some of the mega patch, this backport would be recorded here. |
Apologies for the noise Terry, rebuilding passes. Unsetting versions: 3.3 Is the failure on the build I reported worth opening an issue for? |
I would wait and see if it happens gain or on other buildbots. As I am sure you know, intermittent failures, not reproducible on demand, are nasty. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: