vimeo / graph-explorer

A graphite dashboard powered by structured metrics
http://vimeo.github.io/graph-explorer/
Apache License 2.0
1.06k stars 93 forks source link

Remove submodules and add setup script #100

Closed hollobon closed 10 years ago

hollobon commented 10 years ago

I'm not sure if this is something you'd be interested in, but this patch creates a new graph_explorer package, adds a setup script, and removes the Python submodules and replaces them with setuptools dependencies. The diff is quite noisy mainly due to the submodule removals I think.

This makes graph_explorer nicely self-contained. It also means eggs can be built for easy deployment.

Dieterbe commented 10 years ago

ultimately i just care about being able to install and run GE as easily as possible on any platform that python supports. and also to be able to easily run a "dev install" by cloning from git and where you immediately see the effect of code changes (i.e. change a file -> restart server automatically with new code).

I'm open to adopting better ways to do this. I'm not very familiar with the pypy/setup.py stuff because i've run into compatibility/dependency hell issues with them before, and because python having a few different package managers system all evolving at the same time doesn't help. but maybe i wasn't doing things the right way. mabye nowadays we should adopt virtualenv too?

maybe we should talk about this on irc, i would like to learn how this approach can address my goals in the first paragraph

Dieterbe commented 10 years ago

conceptually, i like it, and want to merge this. however, some remarks:

the README will need updated instructions specifically:

hollobon commented 10 years ago

Sorry about exec flags - that's because I'm on Windows. I'll correct.

Will update the README too.

hollobon commented 10 years ago

OK, I think that covers everything.

Dieterbe commented 10 years ago

something is not quite right. it seems like a few files came into existence out of no where. when I run git diff -M --stat master renshawbay/master, I see for example:

 graph_explorer/structured_metrics/plugins/swift.py                                                            |   48 +
 graph_explorer/structured_metrics/plugins/swift_object_auditor.py                                             |   23 +
 graph_explorer/structured_metrics/plugins/swift_object_server.py                                              |   21 +
 graph_explorer/structured_metrics/plugins/swift_proxy_server.py                                               |   38 +
 graph_explorer/structured_metrics/plugins/swift_tempauth.py                                                   |   18 +
 graph_explorer/structured_metrics/sql_fiddle.txt                                                              |   70 +
hollobon commented 10 years ago

Fixed. I'm not sure how it happened.

Dieterbe commented 10 years ago

and what about the swift plugins?

hollobon commented 10 years ago

6a9e488?

Dieterbe commented 10 years ago

ah yes, i missed that commit, thanks. i'll try this out again.

Dieterbe commented 10 years ago

why the rename of graph-explorer.py to run_graph_explorer.py ?

Dieterbe commented 10 years ago

also, shouldn't we add all these eggs to gitignore?

    Paste-1.7.5.1-py2.7.egg/
    Paste-1.7.5.1-py3.3.egg/
    WTForms-1.0.5-py3.3.egg/
    bottle-0.11.6-py2.7.egg/
    bottle-0.11.6-py3.3.egg/
    elasticsearch-0.4.3-py2.7.egg
    pytest-2.5.2-py3.3.egg/
    setuptools_git-1.0-py2.7.egg
    setuptools_git-1.0-py3.3.egg
    urllib3-1.7.1-py2.7.egg/
hollobon commented 10 years ago

Regarding the rename, I prefer to name Python modules with a name that can be imported. Admittedly you probably won't want to import a script but it could be useful for testing. The "run" just disambiguates from the package name.

I can change back if you prefer.

Dieterbe commented 10 years ago

ah that makes sense

Dieterbe commented 10 years ago

I merged this, thanks very much! so just to clarify, for a development env with virtualenv, is the below a good approach?, an do you see any errors in my comments? (note i need to run virtualenv2 otherwise it uses a python3 one)

$ cd <GE code dir>
$ virtualenv2 ge-dev  # make virtualenv in a subdir of the code dir
$ cd ge-dev
$ source bin/activate
$ python setup.py develop  # knows to install in the virtualenv because activate changed some environment variables
$ run_graph_explorer.py  # this is now in $PATH
#  because ge-dev/lib/python2.7/site-packages/graph-explorer.egg-link points to the code dir, I can just change code and the server in the virtualenv immediately reloads and the changes are visible

also, how would you recommend people do a prod install using virtualenv?

Dieterbe commented 10 years ago

also, when i modify the config, run_graph_explorer.py doesn't reflect this, even when i run python setup.py develop again (after activitating the env), so how are users supposed to use a modified config? also we will need to update the README to reflect this

hollobon commented 10 years ago

Comments look fine.

For a prod install, it depends on circumstances. I would think:

I will have a look at the config thing.

hollobon commented 10 years ago

I haven't had any problems with editing config.py. Give me a shout on IRC (hbn) and we can discuss.

Dieterbe commented 10 years ago

hey @hollobon couldn't reach you on irc. for the reference: in config.py i have es_host set to a certain host, i run 'source ge-dev/bin/activate', which updates my shell prompt, and then execute run_graph_explorer.py but it says it can't connect to elasticsearch @ MaxRetryError(HTTPConnectionPool(host='localhost', port=9200) i can make it work like this: cp config.py ge-dev/lib/python2.7/site-packages/graph_explorer-0.0.0-py2.7.egg/graph_explorer/config.py but this doesn't always work (! very weird?), and that should not be the location of config.py anyway. also timeserieswidget doesn't seem to be able to load anymore this seems to fix it: cp -ax timeserieswidget ge-dev/lib/python2.7/site-packages/graph_explorer-0.0.0-py2.7.egg/graph_explorer/

Dieterbe commented 10 years ago

hmm the config looks like it should go into graph_explorer/config.py but updating that and removing config.py in the main dir is not enough, even after removing ge-dev/lib/python2.7/site-packages/graph_explorer-0.0.0-py2.7.egg/graph_explorer/config.py

it looks like i have to modify graph_explorer/config.py and then run "python2 setup.py install" again. still confused..

hollobon commented 10 years ago

Sorry been on holiday. I will take a look.

hollobon commented 10 years ago

I think the problem here is the mixing of code and config.

I propose replacing config.py with a configuration file parsed by ConfigParser, and passing the path to this config file as an argument to run_graph_explorer.py. This means it's easy to run multiple instances of Graph Explorer from a single virtualenv, too.

I'll knock up a quick patch if you want to see how this would look.