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

Configs now have to be named as proper Python modules #787

Closed
dsoprea opened this Issue Jun 14, 2014 · 7 comments

Comments

Projects
None yet
3 participants
@dsoprea

dsoprea commented Jun 14, 2014

Traditionally, I name my Gunicorn configuration files for the environment that they run in, such as gunicorn.conf.dev or gunicorn.conf.prod. However, as of several hours ago, I started running into this:

Failed to read config file: deploy_ui/resources/data/gunicorn.conf.dev
Traceback (most recent call last):
  File "/Users/dustin/development/python/deployment/deploy_ui/lib/python2.7/site-packages/gunicorn/app/base.py", line 94, in load_config_from_file
    execfile_(filename, cfg, cfg)
  File "/Users/dustin/development/python/deployment/deploy_ui/lib/python2.7/site-packages/gunicorn/six.py", line 390, in execfile_
    return exec_(_get_codeobj(fname), *args)
  File "/Users/dustin/development/python/deployment/deploy_ui/lib/python2.7/site-packages/gunicorn/six.py", line 318, in _get_codeobj
    result, fileobj, fullpath = _check_if_pyc(pyfile)
  File "/Users/dustin/development/python/deployment/deploy_ui/lib/python2.7/site-packages/gunicorn/six.py", line 309, in _check_if_pyc
    raise IOError("Cannot find config file. Path maybe incorrect! : {0}".format(filepath))
IOError: Cannot find config file. Path maybe incorrect! : /Users/dustin/development/python/deployment/deploy_ui/deploy_ui/resources/data/gunicorn.conf.dev
Traceback (most recent call last):
  File "./dui_start_gunicorn_dev", line 20, in <module>
    raise EnvironmentError("Gunicorn launch failed.")
EnvironmentError: Gunicorn launch failed.

I could only fix it by renaming the configuration file to gunicorn_conf_dev.py .

It looks like your change from:

def _check_if_pyc(fname):

to:

def _check_if_pyc(fname):

is what broke it.

Man. I just lost several hours debugging several applications and several combinations of dependency versions trying to figure out why projects worked and then didn't work. It turns out that I had a couple of different Gunicorn versions, and that it was Gunicorn's changes that had broken everything. It took me extra long to identify it because I usually exclude Gunicorn as a suspect.. It never is.

I searched the issue on Google, and got zero relevant results. Meanwhile, I had sought to verify if anything had changed with how to name Gunicorn configurations and how to launch with them, but there weren't any examples in the documentation. I guess I must've picked-up my conventions from some other project back when I first started using Gunicorn.

I highly recommend that you add such an example for using a configuration file and how to name it, and put a warning in the documentation that configurations must now be named like proper Python modules, but didn't previously have to be.

@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Jun 14, 2014

Owner

There is an example in examples: https://github.com/benoitc/gunicorn/blob/master/examples/example_config.py

To be honnest I didn't expect any breaking change here. Gunicorn should still be able to load any file.

It has been introduced by 8b49de4 to solve #693 . We should either fix this change or revert it for the coming release. @jerynmathew would be cool if you could also have a look. I will work on it later in the day.

Owner

benoitc commented Jun 14, 2014

There is an example in examples: https://github.com/benoitc/gunicorn/blob/master/examples/example_config.py

To be honnest I didn't expect any breaking change here. Gunicorn should still be able to load any file.

It has been introduced by 8b49de4 to solve #693 . We should either fix this change or revert it for the coming release. @jerynmathew would be cool if you could also have a look. I will work on it later in the day.

@dsoprea

This comment has been minimized.

Show comment
Hide comment
@dsoprea

dsoprea Jun 14, 2014

It looks like it was a major refactor. That being said, I took a brief glance at the current version of the file to see if there were any quick fixes. _check_if_pyc is being called by _get_codeobj (

result = _check_if_pyc(pyfile)
), but _get_codeobj isn't called from within its own module. Is _get_codeobj being called from another module (which would be weird, given the naming convention)?

dsoprea commented Jun 14, 2014

It looks like it was a major refactor. That being said, I took a brief glance at the current version of the file to see if there were any quick fixes. _check_if_pyc is being called by _get_codeobj (

result = _check_if_pyc(pyfile)
), but _get_codeobj isn't called from within its own module. Is _get_codeobj being called from another module (which would be weird, given the naming convention)?

benoitc referenced this issue Jun 14, 2014

Fix for issue #693
	- Remodeled the logic to use imp module to validate the python
	  gunicorn config file

dsoprea added a commit to dsoprea/RestPipe that referenced this issue Jun 14, 2014

Deployment/naming refactors.
- Moved README.md and requirements.txt to resources/ so that we can reference them from setup.py .
- Began documentation.
- Renamed gunicorn configs so they'll load correctly in light of Gunicorn bug (benoitc/gunicorn#787).
- Fixed packages value in setup.py . Was excluding non-root packages.
@benoitc

This comment has been minimized.

Show comment
Hide comment
@benoitc

benoitc Jun 14, 2014

Owner

@dsoprea i posted a pr to fix it. please let me know. I intend to make a new release today.

Owner

benoitc commented Jun 14, 2014

@dsoprea i posted a pr to fix it. please let me know. I intend to make a new release today.

@dsoprea

This comment has been minimized.

Show comment
Hide comment
@dsoprea

dsoprea Jun 14, 2014

I won't be able to test it for a few hours.

On Sat, Jun 14, 2014 at 4:23 AM, Benoit Chesneau notifications@github.com
wrote:

@dsoprea https://github.com/dsoprea i posted a pr to fix it. please let
me know. I intend to make a new release today.


Reply to this email directly or view it on GitHub
#787 (comment).

dsoprea commented Jun 14, 2014

I won't be able to test it for a few hours.

On Sat, Jun 14, 2014 at 4:23 AM, Benoit Chesneau notifications@github.com
wrote:

@dsoprea https://github.com/dsoprea i posted a pr to fix it. please let
me know. I intend to make a new release today.


Reply to this email directly or view it on GitHub
#787 (comment).

@dsoprea

This comment has been minimized.

Show comment
Hide comment
@dsoprea

dsoprea Jun 16, 2014

Looks good.

dsoprea commented Jun 16, 2014

Looks good.

dsoprea added a commit to dsoprea/RestPipe that referenced this issue Jun 16, 2014

Deployment/naming refactors.
- Moved README.md and requirements.txt to resources/ so that we can reference them from setup.py .
- Began documentation.
- Renamed gunicorn configs so they'll load correctly in light of Gunicorn bug (benoitc/gunicorn#787).
- Fixed packages value in setup.py . Was excluding non-root packages.
@jerynmathew

This comment has been minimized.

Show comment
Hide comment
@jerynmathew

jerynmathew Jun 16, 2014

Contributor

Hello all,

Sorry for the delay to respond.

The issue seems to be the use of imp.find_module(). It insists that the
file be in importable shape - the path, the .py extn, etc. The dots in the
filename (eg: gunicorn.conf.dev) would actually break the python import
within the imp module.

It actually came in after a review comment. Seemed like a good thought at
the time. Sorry for all the unexpected trouble.

@benoitc: Your fix should easily hold up as well. Looks good.

Best Regards,
Jeryn

PS. Please excuse the brevity. I'm on mobile device.
On Jun 16, 2014 8:33 AM, "Dustin Oprea" notifications@github.com wrote:

Looks good.


Reply to this email directly or view it on GitHub
#787 (comment).

Contributor

jerynmathew commented Jun 16, 2014

Hello all,

Sorry for the delay to respond.

The issue seems to be the use of imp.find_module(). It insists that the
file be in importable shape - the path, the .py extn, etc. The dots in the
filename (eg: gunicorn.conf.dev) would actually break the python import
within the imp module.

It actually came in after a review comment. Seemed like a good thought at
the time. Sorry for all the unexpected trouble.

@benoitc: Your fix should easily hold up as well. Looks good.

Best Regards,
Jeryn

PS. Please excuse the brevity. I'm on mobile device.
On Jun 16, 2014 8:33 AM, "Dustin Oprea" notifications@github.com wrote:

Looks good.


Reply to this email directly or view it on GitHub
#787 (comment).

@benoitc benoitc closed this in #789 Jun 16, 2014

benoitc added a commit that referenced this issue Jun 16, 2014

Merge pull request #789 from benoitc/fix/787
fix #787 check if we load a pyc file or not.
@dsoprea

This comment has been minimized.

Show comment
Hide comment
@dsoprea

dsoprea Jun 18, 2014

You mentioned doing a release?

dsoprea commented Jun 18, 2014

You mentioned doing a release?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment