-
Notifications
You must be signed in to change notification settings - Fork 13
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
Fix file uploads on python 3.4 and up. #13
Conversation
cgi.FieldStorage explicitly closes files when it is garbage collected. For details, see: * http://bugs.python.org/issue18394 * https://hg.python.org/cpython/rev/c0e9ba7b26d5 We now keep a reference to the FieldStorage till we are finished processing the request.
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.
LGTM!
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
27 similar comments
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Travis is seriously unhappy on Python 3 for what look like unrelated reasons. Feel free to merge if the tests pass locally for you. (In the past, instead of debugging the setup.py hacks that attempt to hook zope.testruner to |
Apologies for the comment flood. I blame GitHub. |
Wow, that's an impressive amount of comments. Here I thought you just Thanks, I tried out your idea here: It works pretty well and I managed to resist the temptation to type I'll wait a bit for anyone who wants to approve then commit both PR's On Fri, Nov 04, 2016 at 03:37:44AM -0700, Marius Gedminas wrote:
Brian Sutherland |
The tests pass, but they need unreleased versions of zope.app.basicskin (zopefoundation/zope.app.basicskin#1) and zope.app.pagetemplate (zopefoundation/zope.app.pagetemplate#1). We also have a workaround for zopefoundation/zope.mimetype#6 (no issue/fix yet) and the unreleased zopefoundation/zope.publisher#13
cgi.FieldStorage explicitly closes files when it is garbage collected. For details, see:
We now keep a reference to the FieldStorage till we
are finished processing the request.