-
-
Notifications
You must be signed in to change notification settings - Fork 289
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
Documentation for #569 #574
Conversation
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.
Thanks! Looks good to me. The only bit I think needs fixing up is the -c
error explanation, but I left some other nits as well.
docs/buildingpex.rst
Outdated
Note: If you run ``pex -c`` and come across an error similar to | ||
``pex.pex_builder.InvalidExecutableSpecification: Could not find script 'mainscript.py' in any distribution within PEX!``, | ||
double-check your setup.py and ensure that ``mainscript.py`` is included | ||
in your setup's ``scripts`` array. |
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.
It would probably be helpful to reference https://python-packaging.readthedocs.io/en/latest/command-line-scripts.html.
It's also probably good to mention console_scripts. You can get the same error for those. Using the 1st example with a -c
value typo:
pex Fabric -c drab -- --help
Traceback (most recent call last):
File ".bootstrap/_pex/pex.py", line 351, in execute
File ".bootstrap/_pex/pex.py", line 281, in _wrap_coverage
File ".bootstrap/_pex/pex.py", line 313, in _wrap_profiling
File ".bootstrap/_pex/pex.py", line 394, in _execute
File ".bootstrap/_pex/pex.py", line 505, in execute_entry
File ".bootstrap/_pex/pex.py", line 527, in execute_pkg_resources
File "/home/jsirois/.pex/install/pex-1.4.7-py2.py3-none-any.whl.e848015a2d871be4f78a2013cb4db3791997343f/pex-1.4.7-py2.py3-none-any.whl/pex/bin/pex.py", line 734, in main
pex_builder = build_pex(reqs, options, resolver_options_builder)
File "/home/jsirois/.pex/install/pex-1.4.7-py2.py3-none-any.whl.e848015a2d871be4f78a2013cb4db3791997343f/pex-1.4.7-py2.py3-none-any.whl/pex/bin/pex.py", line 677, in build_pex
pex_builder.set_script(options.script)
File "/home/jsirois/.pex/install/pex-1.4.7-py2.py3-none-any.whl.e848015a2d871be4f78a2013cb4db3791997343f/pex-1.4.7-py2.py3-none-any.whl/pex/pex_builder.py", line 225, in set_script
script, ', '.join(str(d) for d in self._distributions)))
pex.pex_builder.InvalidExecutableSpecification: Could not find script 'drab' in any distribution six 1.11.0, bcrypt 3.1.4, pycparser 2.19, paramiko 2.4.2, cffi 1.11.5, PyNaCl 1.3.0, pyasn1 0.4.4, idna 2.7, invoke 1.2.0, cryptography 2.3.1, asn1crypto 0.24.0, fabric 2.4.0 within PEX!
Even better would be to just inline this fix in the code - there are 2 uses:
https://github.com/pantsbuild/pex/blob/cbf690845d40da54b3c7a3c2704c55a4a9dc614b/pex/bin/pex.py#L676-L677
https://github.com/pantsbuild/pex/blob/cbf690845d40da54b3c7a3c2704c55a4a9dc614b/pex/commands/bdist_pex.py#L38-L39
These are the only two entrypoints for running (1st supports pex ...
like you're used to, 2nd supports python setup.py bdist_pex
); so it's appropriate to convert from an exception to a clean die at those points.
It would be as simple as:
try:
builder.set_script(script)
except InvalidExecutableSpecification e:
die("....useful message like you have here with python packaging pointer...")
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.
Interesting, I didn't realize there was a way to invoke specific functions that way, that's super cool. I'll update the note to include the console_scripts approach and a link to the packaging docs
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.
Interesting, I didn't realize there was a way to invoke specific functions that way
Nitpicking your words to make sure things are crystal clear:
You can specify an entry point in 3 ways when building a pex:
-c <named console script>
: these are entrypoint functions granted names in setup.py-c <script>
: these are the basenames of actual scripts in setup.py-e <entrypoint>
: this is an arbitrary entrypoint function; ie:myapp.submodule:nullary_function_name
When building a pex, method 3 is the only way to have a specific arbitrary function invoked. Method 1 only allows pre-named functions ("console scripts") to be invoked.
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.
I appreciate the clarity, I've been going on this project in a breadth-first way so it's nice to have that spelled out
docs/recipes.rst
Outdated
PEX Recipes and Notes | ||
===================== | ||
|
||
This is a space for info beyond the purpose or building of PEX files. |
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.
I think you can probably discard this line, the title above is clear.
docs/recipes.rst
Outdated
the PEX from invoking Gunicorn. | ||
|
||
This retains the benefit of zero `pip install`'s to run your service, but it | ||
requires a bit more setup. First, ensure Gunicorn is packaged as a depenency, |
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.
Probably break the sentence: ...as a dependency. The following...
docs/recipes.rst
Outdated
|
||
$ pex flask gunicorn myapp -e gunicorn.app.wsgiapp -o ~/service.pex | ||
|
||
The dependency on Gunicorn can live in your setup.py as well: |
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.
Although true, if the app has 0 lines of code that import from gunicorn this is not a clean recommendation. Consider dropping.
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.
Fair point, it is a bit of a hack. I added the option as it's a cleaner approach for build systems. In terms of code, the app definitely doesn't rely on Gunicorn - especially with Flask included as well - but from an ops view Gunicorn is required for the app to go live, and it's easier to have a standard PEX invocation at compile time that leans more on setup.py than to have special build configuration for each wsgi app.
Would it be more appropriate to shorten that bit to just line 27, and instead of a code block, have a caveat that it may make CI/CD easier but is not recommended when the app isn't using Gunicorn imports?
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.
Also a fair point, but I think at the wrong scope for these docs. I'd leave it out. You've already taught the reader how to use pex to create a self-contained pex with gunicorn / wsgi container included.
If they want to use a pants BUILD file with a python_binary
definition with the extra deps, write a side-car setup.py, a setup.py that depends on the original, or even modify the original setup.py except use extras_require
instead of install_requires
(say with an extra requirement list named 'deployable': pex myapp[deployable] -e gunicorn.app.wsgiapp -o ~/service.pex
), then they can probably figure that out.
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.
Great points, I dig it - redacted that section
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.
Thanks a bunch for working through this. One last small bit and I'll merge.
docs/recipes.rst
Outdated
|
||
.. code-block:: bash | ||
|
||
$ pex flask gunicorn myapp -e gunicorn.app.wsgiapp -o ~/service.pex |
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.
Sorry about this - one last edit please: pex flask gunicorn myapp -c gunicorn -o ~/service.pex
.
I do not know gunicorn and the -e
seemed suspiciously manual for something that is presumably gunicorn bread and butter. My discovery process:
$ pex gunicorn -o gunicorn.pex
$ unzip -qc gunicorn.pex .deps/gunicorn-19.9.0-py2.py3-none-any.whl/gunicorn-19.9.0.dist-info/entry_points.txt
[console_scripts]
gunicorn=gunicorn.app.wsgiapp:run
gunicorn_paster=gunicorn.app.pasterapp:run
[paste.server_runner]
main=gunicorn.app.pasterapp:paste_server
$ unzip -qc gunicorn.pex .deps/gunicorn-19.9.0-py2.py3-none-any.whl/gunicorn/app/wsgiapp.py | tail
"""\
The ``gunicorn`` command line runner for launching Gunicorn with
generic WSGI applications.
"""
from gunicorn.app.wsgiapp import WSGIApplication
WSGIApplication("%(prog)s [OPTIONS] [APP_MODULE]").run()
if __name__ == '__main__':
run()
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.
Oh... I spent an embarrassing amount of time researching/asking questions regarding executing a specific module under gunicorn. And I'm glad it can be this simple. I hadn't seen a line of docs on entry_points
until our chat here, even after digging deep into the python docs on setuputils. Now knowing about it, it's aggressively obvious in the Gunicorn setup file 🤕
This PR has been a journey of enlightenment.
So that totally works like a charm and simplifies a huge pile of assumptions I've had about the way I've been hot-wiring Gunicorn. Update commit inbound.
Thanks again @jakethedev! |
No prob, thanks for filling in the blanks for some of this stuff! |
This is really good. But wondering if there is a complete beginners example using pants/pex, flask and gunicorn? Because I have been building my flask app like this |
Hey @kavink - gunicorn can't handle running a zip file (to my knowledge), so you have to bundle flask and gunicorn with your application at build time for it to work. This can be done in-line with pex, or by adding gunicorn and flask directly to the install_requires of your setup.py - but I'm not sure what that looks like in a pants build situation From there, I don't know how pants sets the entry point for the application, but you'll want to set it so it's |
Adding information on gunicorn with pex, plus a note on a common error to encounter with
pex -c