Skip to content
This repository has been archived by the owner on Apr 23, 2024. It is now read-only.

Can we make this module installable via pip? #74

Open
lipis opened this issue Dec 19, 2014 · 19 comments
Open

Can we make this module installable via pip? #74

lipis opened this issue Dec 19, 2014 · 19 comments

Comments

@lipis
Copy link
Contributor

lipis commented Dec 19, 2014

No description provided.

@houmie
Copy link

houmie commented Dec 19, 2014

Totally agree, at this time and age, every project should support pip.
I just tried it via
pip install https://github.com/Khan/gae_mini_profiler/archive/master.zip
and it fails.
same with
pip install git+https://github.com/Khan/gae_mini_profiler.git

Please!

@kolyaflash
Copy link

I think we should replace "make installable" with "make downloadable" in case with GAE and taking into account how gae_mini_profiler integrates with app.
And it sounds pretty senseless.

@houmie
Copy link

houmie commented Dec 19, 2014

I think it would be very useful. Have a look at template projects such as https://github.com/kamalgill/flask-appengine-template or https://github.com/gae-init/gae-init, you will see that they use virtualenv for all packages. Personally I utilise make setup to install via pip and rsync the files to the lib folder. That way I keep the Lib clean of unnecessary egg files.

Hence having the option to install it via pip would be incredibly useful.

@bogdanvarlamov
Copy link

All it needs is a setup.py file, to be supported by pip, right?

@jlfwong
Copy link
Contributor

jlfwong commented Nov 11, 2015

If it were installable via pip, how would the static assets be used? i.e. how would you specify the path app.yaml to gae_mini_profiler/static?

@lipis
Copy link
Contributor Author

lipis commented Nov 11, 2015

By installing it to a specific folder?

pip install --install-option="--prefix=/path/to/gae/app" gae_mini_profiler

@jlfwong
Copy link
Contributor

jlfwong commented Nov 11, 2015

Is that option specifiable via a requirements.txt?

@lipis
Copy link
Contributor Author

lipis commented Nov 11, 2015

hmm.. actually I'm using it here: https://github.com/gae-init/gae-init-debug/tree/master/main/libx and the whole magic with pip was done by @gmist so maybe he can help more in this area :)

@jlfwong
Copy link
Contributor

jlfwong commented Nov 11, 2015

My gut feeling is that this isn't particularly useful unless it's specifiable via requirements.txt. If you have to do a manual pip install step and specify a target directory, then I'm not seeing much by way of significant benefit over using git clone or using a submodule.

For reference, at Khan Academy, we use it as a submodule of our main repository.

@bogdanvarlamov
Copy link

I'm installing everything from requirements.txt into a lib folder that I
don't want in version control. Currently I have to manually add the gae
mini profiler from git into this lib folder, or else have inconsistent
import paths.

If it were a pip installable module, I wouldn't have to do this hacky
workflow.

On Wed, Nov 11, 2015, 12:57 PM Jamie Wong [email protected] wrote:

My gut feeling is that this isn't particularly useful unless it's
specifiable via requirements.txt. If you have to do a manual pip install
step and specify a target directory, then I'm not seeing much by way of
significant benefit over using git clone or using a submodule.

For reference, at Khan Academy, we use it as a submodule of our main
repository.


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

@lipis
Copy link
Contributor Author

lipis commented Nov 11, 2015

Same here.. as everything else for GAE, the 3rd party libs needs to be included in the project's directory.. git modules are nice, but not very practical in real life :)

@houmie
Copy link

houmie commented Nov 11, 2015

Yes, this is exactly my workflow as well. I have to copy it in manually into lib folder, and I don't want the .git folder in there.

@jlfwong
Copy link
Contributor

jlfwong commented Nov 11, 2015

Yes, this is exactly my workflow as well. I have to copy it in manually into lib folder, and I don't want the .git folder in there.

Sorry if I'm misunderstanding, but this isn't what I'm suggesting -- this is the role of git submodule.

In our main repository, for instance, we do this:

git submodule add [email protected]:Khan/gae_mini_profiler.git third_party/gae_mini_profiler

I'm installing everything from requirements.txt into a lib folder that I don't want in version control.

If I'm understanding correctly, this means you have pip install --install-option="--prefix=/path/to/gae/app", which installs all packages listed in requirements.txt as part of your dev environment setup + part of your deploy scripts? If that's the case, then I can see making this repo pip installable as pretty reasonable.

If that's the path that you're recommending, then the next step would be if someone would be so kind as to make a setup.py for this repo along with relevant documentation in the README indicating that you must use this in conjunction with --install-option, and submit that as a pull request.

@bogdanvarlamov
Copy link

Jamie,

You are exactly correct in how I'm using it (pip command as part of
setup/deploy).

I've created a fork where I'm experimenting with creating the setup.py and
manifest, etc.

I'll send a pull request if I can make it work. So far I can create an
archive with ask the necessary files, so I'm close.

On Wed, Nov 11, 2015, 4:59 PM Jamie Wong [email protected] wrote:

Yes, this is exactly my workflow as well. I have to copy it in manually
into lib folder, and I don't want the .git folder in there.

Sorry if I'm misunderstanding, but this isn't what I'm suggesting -- this
is the role of git submodule.

In our main repository, for instance, we do this:

git submodule add [email protected]:Khan/gae_mini_profiler.git third_party/gae_mini_profiler

I'm installing everything from requirements.txt into a lib folder that I
don't want in version control.

If I'm understanding correctly, this means you have pip install
--install-option="--prefix=/path/to/gae/app", which installs all packages
listed in requirements.txt as part of your dev environment setup + part
of your deploy scripts? If that's the case, then I can see making this repo
pip installable as pretty reasonable.

If that's the path that you're recommending, then the next step would be
if someone would be so kind as to make a setup.py for this repo along
with relevant documentation in the README indicating that you must use
this in conjunction with --install-option, and submit that as a pull
request.


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

@lipis
Copy link
Contributor Author

lipis commented Nov 12, 2015

Good stuff..! Looking forward @bogdanvarlamov.

@jlfwong what about the other 3rd party libraries?! Are you not using pip for them Khan Academy?

(In general it makes sense for you guys to have a git submodule as this is your library and you know what is going on, but depending on a git submodule in other projects makes it inconvenient to work, especially when it comes to versions. If you have everything in the requirements.txt it's much easier..)

@jlfwong
Copy link
Contributor

jlfwong commented Nov 12, 2015

@jlfwong what about the other 3rd party libraries?! Are you not using pip for them Khan Academy?

We are, but we have a bunch of restrictions imposed as a result of GAE's system for libraries: https://cloud.google.com/appengine/docs/python/tools/libraries27?hl=en and we run gae-mini-profiler (as well as many other third party libs) in production, so we can't use pip for those.

pip, for us, is only used for dev-only tools, and for things that happen to be a subset of the libraries listed on that page.

@bogdanvarlamov
Copy link

Hi All,

I created a branch in my fork of this repo that contains the refactoring needed to have this project be "pipable". It is available here: https://github.com/bogdanvarlamov/gae_mini_profiler/tree/pip_support

Since the changes are more extensive than I initially thought, I need @jlfwong to take a look and decide what is preferable:

  1. pull the changes and re-organize the layout of this project to match what pip packages should be structured as
    -- if so, I need to know who/what to put in the license file for (c) info as well as the version number for the package (and author info for pip package as well)
  2. create a separate pip-ready fork to be maintained apart from this main repo (maybe this repo can even be a submodule in that repo)

If anyone is curious about trying out my fork in their projects to make sure it works right, here are the steps:

  1. Add this line to your requirements_dev.txt file:
    git+https://github.com/bogdanvarlamov/gae_mini_profiler.git@pip_support#egg=gae_mini_profiler
  2. Install all dependencies as you normally would via pip install -r requirements_dev.txt -t src/lib

You can now import gae_mini_profiler packages into your code just like you would with any other package in the lib folder.

@benjaminjkraft
Copy link
Contributor

@bogdanvarlamov Thanks for looking into this!

It looks like as-written this will require some changes to projects already using the mini profiler, because of the directory change. Do you know if there's a reasonable way to make it pip-installable without doing that, even if it's not exactly the standard/recommended approach? Doing the update is fairly easy for us, but I'd be concerned about breaking things for other sites that use it, especially since we don't really have any versioning at the moment. If we can do it without breaking things, we'd certainly be happy to accept a pull request and we can talk about the details of that then.

@bogdanvarlamov
Copy link

Unfortunately I spent a good bit of time trying different hacks to get a
pip package without directory structures, but I couldn't get it to work.

Keep in mind that I've never made a pip install package before, and don't
use Python daily, so I might just be an idiot and missing something an
experienced person might know ;)

Out of the hacks I tried, the only plausibly working one was to explicitly
list all files and where they must be copied, but it was so awful I
abandoned that approach due to maintainability concerns.

On Tue, Dec 8, 2015, 8:29 PM Ben Kraft [email protected] wrote:

@bogdanvarlamov https://github.com/bogdanvarlamov Thanks for looking
into this!

It looks like as-written this will require some changes to projects
already using the mini profiler, because of the directory change. Do you
know if there's a reasonable way to make it pip-installable without doing
that, even if it's not exactly the standard/recommended approach? Doing the
update is fairly easy for us, but I'd be concerned about breaking things
for other sites that use it, especially since we don't really have any
versioning at the moment. If we can do it without breaking things, we'd
certainly be happy to accept a pull request and we can talk about the
details of that then.


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

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

No branches or pull requests

6 participants