Fix for #693 #696
Fix for #693 #696
Conversation
- Made changes to read and fetch code object from .pyc files. - Error message "Non-ASCII character '\xf3' in file" is resolved.
- Override older behavior of execfile_ with one with pyc patch
| @@ -286,6 +286,50 @@ def StringIO(buf=''): | |||
| _add_doc(u, """Text literal""") | |||
|
|
|||
|
|
|||
| def _check_if_pyc(fname): | |||
There was a problem hiding this comment.
I think this might be a lot more readable if we used splitext
There was a problem hiding this comment.
Actually, maybe we can use find_module and this wouldn't even need to check the suffix.
There was a problem hiding this comment.
looks like imp.load_module is also able to load compiled files so a combination of both should probably work. @jerynmathew did you tried to use them already?
There was a problem hiding this comment.
Hi @benoitc,
Sorry for the delay, busy at my day job.
I tested the imp.load_module(); it returns a module-object type.
The exec (or execfile) expects string, file or code-object type. Hence I'm not sure imp.load_module() is the right choice here, because once we obtain the module object from imp.load_module(), we still need to convert it to code object (which is done as shown in my code).
There was a problem hiding this comment.
That's why I recommended only find_module.
There was a problem hiding this comment.
I made a commit again. Hope this is alright!
There was a problem hiding this comment.
Thanks. I think that is easier to read. I particularly like the constants
instead of the file extensions.
It's unlikely to be a big problem in this case, but it would be good style
to bring the read call up above the conditional, wrap it in a try/finally
and close the file in the finally block.
As is, you duplicate the read and close call and don't guarantee to close
if there's an exception.
Could you make that change and then I'll merge this?
In gunicorn/six.py:
@@ -286,6 +286,50 @@ def StringIO(buf=''):
_add_doc(u, """Text literal""")+def _check_if_pyc(fname):
I made a commit again. Hope this is alright!
—
Reply to this email directly or view it on
GitHubhttps://github.com//pull/696/files#r10633817
.
There was a problem hiding this comment.
Good catch! Sorry about the rookie mistake.
All done now.
There was a problem hiding this comment.
Not a problem at all! Thanks for the update.
- Remodeled the logic to use imp module to validate the python gunicorn config file
- Added changes to file read, as per review comments
This fix will now accept .pyc files for config input from command line.
ie, gunicorn -c config.pyc app:main
This will greatly help production environments where python source are never shipped, only compiled ones.