Skip to content
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

google-app-engine-python-sdk: init at 1.9.35 #14237

Closed
wants to merge 1 commit into from

Conversation

chris-martin
Copy link
Contributor

There are two new packages here:

  1. pkgs.python27Packages.google-app-engine-sdk
  2. pkgs.google-app-engine-python-sdk

The first package is just unmodified SDK, useful if all you want to do is import the modules from python code. The second package provides commands to run the app-engine tools (for example, google-app-engine-python-dev-appserver to run dev_appserver.py).


  • Tested using sandboxing (nix-build --option build-use-chroot true or nix.useChroot on NixOS)
  • Built on platform(s)
    • NixOS
    • OS X
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@@ -0,0 +1,24 @@
{ pkgs }:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is poor style, make dependencies explicit by passing them as individual parameters.

@joachifm
Copy link
Contributor

Please de-compose this into separate commits for each change.

@chris-martin
Copy link
Contributor Author

I've addressed all of @joachifm's comments except the request to comment why google-app-engine-python-sdk doesn't use buildPythonApplication. I'm not sure how to answer this question, because I can't figure out why one would consider using buildPythonApplication to create these scripts.

I suppose I could create a setup.py for this, but it seems backwards to introduce setuptools into a project that doesn't use it?

@joachifm
Copy link
Contributor

Okay, I didn't look too closely at what you were doing

@chris-martin
Copy link
Contributor Author

Sorry, I guess this calls for some additional background explanation.

App Engine is kind of weird as far as Python stuff goes. It provides some core libraries and expects the rest of your dependencies to be included in your app, so you don't set up PYTHONPATH for it. When you deploy to App Engine in production, you ship an artifact that contains your code as well as any third-party libraries you're using. The dev appserver mirrors that process; when you run dev_appserver.py path-to-your-app, that script is responsible for setting up PYTHONPATH to include your app as well as the libraries that are built into the App Engine platform.

The SDK isn't packaged as a Python library (no setup.py) because it's mostly designed to be used just by unzipping the SDK and running the scripts in it. I shoved it into pythonPackages here, though, because there are a few instances where it makes sense to treat it that way. For example, gae-secure-scaffold comes with a test Python script that imports dev_appserver to call fix_sys_path().

Maybe some/all of this belongs in comments?

@chris-martin
Copy link
Contributor Author

I may need some help figuring out how to fix this Travis failure. 😦

The log length has exceeded the limit of 4 MB (this usually means that the test suite is raising the same exception over and over).

The job has been terminated

@joachifm
Copy link
Contributor

There's nothing to be done about it. Please do perform a sandboxed build on your own machine or wait for someone to do it for you.

I think @FRidh may provide some useful feedback.

@chris-martin
Copy link
Contributor Author

Okay, I have done a sandboxed build, just wanted to make sure the Travis failure isn't indicative of a real problem.

@@ -0,0 +1,24 @@
{ pkgs, stdenv, python27Full, python27Packages, makeWrapper }:
with {
python = python27Full;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you need all of the extra modules? Would it be possible to instead pick only the exact modules you need?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As far as I know, we only need sqlite3. Is there a way to construct a python that only has this module? I see the includeModules boolean, but I don't see an arg to specify a particular set of modules.

@FRidh
Copy link
Member

FRidh commented Mar 28, 2016

buildPythonPackage and buildPythonPackage are for packages that use setuptools. Some Python packages need to be installed differently, and using mkDerivation is the correct approach then. I see you've explained it also in the code.

I understand why you made it into two separate packages and it kind of makes sense but I don't think it is necessary.

While we discourage installing Python packages, installing a package to obtain its binaries/scripts does work. For example, nix-env -iA pythonPackages.ipython results in a working interpreter.

In this case, having just a pythonPackages.google-app-engine-sdk would be enough. It can be used as a library (e.g. with nix-shell) and by installing it in a profile the items in /bin can be used. You will have to wrap them like you already did. Whether you then rename them (or link with other names) is up to you.

@chris-martin
Copy link
Contributor Author

I understand why you made it into two separate packages and it kind of makes sense but I don't think it is necessary.

Okay. I like breaking things into pieces as small as possible, but I can appreciate the desire to have fewer packages too.

@chris-martin
Copy link
Contributor Author

@FRidh I've combined the packages into one as requested. I still haven't figured out how to remove the dependency on python27Full without removing the sqlite3 module, though. Are there any existing examples in the repo to follow?

@FRidh
Copy link
Member

FRidh commented Mar 29, 2016

@chris-martin see http://nixos.org/nixpkgs/manual/#sec-python
You can add python.modules.sqlite3 to propagatedBuildInputs in case of CPython 2.x

@chris-martin
Copy link
Contributor Author

When I do that, app engine fails to launch.

Traceback (most recent call last):
  File "/nix/store/3gggs6rswzw6x02jwv664vj1c2mqm8nx-google-app-engine-sdk-1.9.35/lib/python2.7/site-packages/dev_appserver.py", line 83, in <module>
    _run_file(__file__, globals())
  File "/nix/store/3gggs6rswzw6x02jwv664vj1c2mqm8nx-google-app-engine-sdk-1.9.35/lib/python2.7/site-packages/dev_appserver.py", line 79, in _run_file
    execfile(_PATHS.script_file(script_name), globals_)
  File "/nix/store/3gggs6rswzw6x02jwv664vj1c2mqm8nx-google-app-engine-sdk-1.9.35/lib/python2.7/site-packages/google/appengine/tools/devappserver2/devappserver2.py", line 35, in <module>
    from google.appengine.tools.devappserver2 import api_server
  File "/nix/store/3gggs6rswzw6x02jwv664vj1c2mqm8nx-google-app-engine-sdk-1.9.35/lib/python2.7/site-packages/google/appengine/tools/devappserver2/api_server.py", line 51, in <module>
    from google.appengine.api.logservice import logservice_stub
  File "/nix/store/3gggs6rswzw6x02jwv664vj1c2mqm8nx-google-app-engine-sdk-1.9.35/lib/python2.7/site-packages/google/appengine/api/logservice/logservice_stub.py", line 24, in <module>
    import sqlite3
  File "/nix/store/2mh0qqaqpd53yql19fqm1fq4cy73f8ah-python-2.7.11/lib/python2.7/sqlite3/__init__.py", line 24, in <module>
    from dbapi2 import *
  File "/nix/store/2mh0qqaqpd53yql19fqm1fq4cy73f8ah-python-2.7.11/lib/python2.7/sqlite3/dbapi2.py", line 28, in <module>
    from _sqlite3 import *
ImportError: No module named _sqlite3

@chris-martin
Copy link
Contributor Author

I'm finding that limitations in wrapPythonPrograms are making this hard.

  • It doesn't expose its ability to recursively build PYTHONPATH as a function
  • Unlike makeWrapper, it can't place the wrapper program at a different path than the original
  • It only works on files with python in the shebang, which means I can't use it to wrap bash scripts (such as ones generated by makeWrapper)

@FRidh
Copy link
Member

FRidh commented Mar 31, 2016

wrapPythonPrograms is intended for buildPythonPackage and buildPythonApplication. It indeed works only with scripts that have python in the shebang.
https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/python-modules/generic/wrap.sh

Because you're not using buildPythonPackage you might want to use pythonPath instead of, or additionally to, propagatedBuildInputs to get sqlite3 working.

@FRidh FRidh added the 2.status: work-in-progress This PR isn't done label Apr 11, 2016
@chris-martin
Copy link
Contributor Author

I made a little progress by explicitly adding the sqlite3 module to PYTHONPATH, but that led another import failure (time). Appengine does some weird stuff with the python path, and I'm afraid it's a bit of a hornets' nest. I've given up.

This PR is working as is. The only remaining issue was that it depends on python27Full. Personally I'm in favor of merging it, as it seems better than nothing to me. @FRidh, please let me know which way you'd like to go.

@chris-martin
Copy link
Contributor Author

The trouble I've had minimizing the dependency set may be related to #492?

@chris-martin
Copy link
Contributor Author

Ping.

This is not a work in progress. If nobody wants to merge this, let me know and I'll sadly close it.

google-app-engine-sdk =
if !isPy27
then throw "google-app-engine-sdk requires Python 2.7; it is not supported for interpreter ${python.executable}"
else pkgs.stdenv.mkDerivation rec {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can use mkPythonDerivation now

# the SDK provides. For example, `dev_appserver.py` becomes
# `google-app-engine-python-dev-appserver`.
for f in $(find $out/lib/${python.libPrefix}/site-packages -maxdepth 1 -type f -regex ".*/[a-z][^/]*.py"); do
makeWrapper $f \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you won't need to call makeWrapper any more then, but likely just want to create symbolic links

for f in $(find $out/lib/${python.libPrefix}/site-packages -maxdepth 1 -type f -regex ".*/[a-z][^/]*.py"); do
makeWrapper $f \
$out/bin/google-app-engine-python-$(basename $f | sed s/_/-/ | sed s/\.py//) \
--prefix PATH : ${pkgs.python27Full}/bin
Copy link
Member

@FRidh FRidh Nov 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in any case, don't use another Python interpreter than the one that's in the package set, so use python, not pkgs.pythonXX. Nowadays the default interpreter includes all modules except tkinter. If you do need tkinter, add it to the propagatedBuildInputs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Google App Engine only works with Python 2.7.9 (and now also 3.5.2).

sha256 = "19qxkxvb7nxs64mdjlxdhzbg16n5qwp8v229lvkpw9w0i8hghpj0";
};

unpackPhase = "unzip $src";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pkgs.fetchzip ?

@ben-z
Copy link
Contributor

ben-z commented Mar 25, 2017

@chris-martin Any updates on this? Really want to use it 😄

@chris-martin
Copy link
Contributor Author

@ben-z It does work, so you can merge it into your own nixpkgs fork if you want to use it.

@FRidh
Copy link
Member

FRidh commented Mar 25, 2017

If I understand correctly this sdk doesn't provide any importable module, is therefore not a Python library but an application and thus does not belong in python-packages.nix but elsewhere in the Nixpkgs tree.

Edit:

So @chris-martin explained it:

Sorry, I guess this calls for some additional background explanation.

App Engine is kind of weird as far as Python stuff goes. It provides some core libraries and expects the rest of your dependencies to be included in your app, so you don't set up PYTHONPATH for it. When you deploy to App Engine in production, you ship an artifact that contains your code as well as any third-party libraries you're using. The dev appserver mirrors that process; when you run dev_appserver.py path-to-your-app, that script is responsible for setting up PYTHONPATH to include your app as well as the libraries that are built into the App Engine platform.

The SDK isn't packaged as a Python library (no setup.py) because it's mostly designed to be used just by unzipping the SDK and running the scripts in it. I shoved it into pythonPackages here, though, because there are a few instances where it makes sense to treat it that way. For example, gae-secure-scaffold comes with a test Python script that imports dev_appserver to call fix_sys_path().

Maybe some/all of this belongs in comments?

@chris-martin
Copy link
Contributor Author

chris-martin commented Nov 19, 2017

I don't do google cloud stuff anymore, but from what I can tell their Python SDK has been replaced by "google-cloud-sdk" (which we do have a nix package for), so this PR seems obsolete.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants