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

Configuration & Server Overhaul - Tool Shed and Reports #3179

Merged
merged 15 commits into from Aug 12, 2017

Conversation

@jmchilton
Copy link
Member

commented Nov 16, 2016

  • Bring startup scripts and config processing inline between Galaxy, Reports, and the Tool Shed (retry of #2837).
  • Define a schema format and tooling (derived from pykwalify) to define web server and application option documentation and validation.
  • Update Galaxy common startup functions to allow starting up uwsgi.
  • Update Galaxy apps to allow YAML-based configuration files.
  • Switch out-of-the box server for use by tool shed and reports to uwsgi.
  • Add schema validation for uwsgi options (derived from uwsgi docs).
  • Add validation/documentation schema for tool shed including
    • Utilities to convert of .ini configs to new YAML ones and switch paste to uwsgi (make tool-shed-config-convert and make tool-shed-config-convert-dry-run).
    • Build a new YAML sample that targets uWSGI out of the box.
    • Utility to validate a tool shed configuration file in the new YAML format (make tool-shed-config-validate).
    • Utility to check best practice config option for tool shed (make tool-shed-config-lint).
  • Add validation/documentation schema for reports including
    • Utilities to convert of .ini configs to new YAML ones and switch paste to uWSGI (make reports-config-convert and make reports-config-convert-dry-run).
    • Build a new YAML sample that targets uWSGI out of the box.
    • Utility to validate a tool shed configuration file in the new YAML format (make reports-config-validate).
    • Utility to check best practice config option for reports (make reports-config-lint).
  • Add reports documentation to the admin section and documentation of the configuration options derived from the schema.

There is no additional duplication between various items (before a .ini.sample had a default and description) now the schema defines these items and everything else (docs, validation, sample YAML) are derived from that. The new validation format should be more usable from within the actual applications themselves to replace existing type conversion and default handling logic - an exciting future direction for this work.

This is hopefully a test bed to bring these concepts quickly to Galaxy - I think all of the infrastructure needed to do this is now readily available after this PR.

This is another attempt at #2866. This extends that work with more unit tests, new linting options, and correctly handles proxy prefix and gzip filters previously expressed in Paste.

@natefoo

This comment has been minimized.

Copy link
Member

commented Nov 16, 2016

Grr Jenkins... I wanted to merge this.

@martenson

This comment has been minimized.

Copy link
Member

commented Nov 16, 2016

@natefoo it targets 17.01, also please let me retest.

@@ -6,6 +6,7 @@ SQLAlchemy==1.0.15
mercurial==3.7.3
numpy==1.9.2
pycrypto==2.6.1
pyuwsgi==2.1.dev0+gx1.c6ee1f6

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Nov 16, 2016

Member

Shouldn't this be named uwsgi as in PyPi? https://pypi.python.org/pypi/uWSGI

This comment has been minimized.

Copy link
@natefoo

natefoo Nov 18, 2016

Member

I believe the naming difference was necessary to use the custom (sadface) version we had to build to make it manylinux wheel-able. More in galaxyproject/starforge#95 and galaxyproject/starforge#114.

This comment has been minimized.

Copy link
@nsoranzo

This comment has been minimized.

Copy link
@natefoo

natefoo Nov 18, 2016

Member

Hrm, yeah, possibly. Since @jxtx's original work in galaxyproject/starforge#95 may have been destroyed I'm not 100% clear.

This comment has been minimized.

Copy link
@jmchilton

jmchilton Nov 21, 2016

Author Member

@nsoranzo I appreciate you bringing this and I don't know the answer to the naming question. Given the difficulty of this - do you think resolving that is important prior to merging this? I think we can just create an issue to track this and go with what is here for now - but obviously we will wait if you think we need to.

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Nov 21, 2016

Member

@jmchilton Not a blocker for me, a tracking issue seems fair!

This comment has been minimized.

Copy link
@natefoo

natefoo Nov 21, 2016

Member

Not a blocker for me either.

AFAIK the way @jxtx is building it is the only way a wheeled uWSGI will work at all and I believe it also needs to be the development version (2.1). Which means that galaxyproject/starforge#114 needs to have both the name and version updated? Unless the changes needed were merged back to 2.0.

@natefoo natefoo referenced this pull request Nov 18, 2016

Closed

New uswgi build #114

@martenson

This comment has been minimized.

Copy link
Member

commented Dec 2, 2016

closing and reopening to trigger selenium tests

@martenson martenson closed this Dec 2, 2016

@martenson martenson reopened this Dec 2, 2016

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 3, 2016

@galaxybot test this

@natefoo

This comment has been minimized.

Copy link
Member

commented Dec 7, 2016

Ok to merge @jmchilton?

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2016

Of course

@martenson

This comment has been minimized.

Copy link
Member

commented Dec 7, 2016

Am I missing something that has to be done after the conversion to yml for uwsgi to work (reports)?

Activating virtualenv at .venv
[uWSGI] getting YAML configuration from config/reports.yml
*** Starting uWSGI 2.1-dev (64bit) on [Wed Dec  7 15:49:14 2016] ***
compiled with version: 4.2.1 Compatible Apple LLVM 7.0.0 (clang-700.0.72) on 28 June 2016 12:17:28
os: Darwin-15.6.0 Darwin Kernel Version 15.6.0: Mon Aug 29 20:21:34 PDT 2016; root:xnu-3248.60.11~1/RELEASE_X86_64
nodename: martenson.bx.psu.edu
machine: x86_64
clock source: unix
detected number of CPU cores: 4
current working directory: /Users/marten/devel/git/galaxy
detected binary path: /Users/marten/devel/git/galaxy/.venv/bin/python
!!! no internal routing support, rebuild with pcre support !!!
your processes number limit is 709
your memory page size is 4096 bytes
detected max file descriptor number: 4864
lock engine: OSX spinlocks
thunder lock: disabled (you can enable it with --thunder-lock)
bind(): Can't assign requested address [core/socket.c line 771]
@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2016

@martenson Can I see the config file?

@martenson

This comment has been minimized.

Copy link
Member

commented Dec 7, 2016

$cat reports.yml
uwsgi:

  http: '127.0.0.1:9001'
  threads: 10

  http-raw-body: true

  offload-threads: 8

  module: 'galaxy.webapps.reports.buildapp:uwsgi_app()'

reports:

  database_connection: 'postgres://usr:pswd@localhost:5432/galaxy-git'
@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2016

@martenson

You already have that address in use?

If not, can you replace http: '127.0.0.1:9001' with http: 'localhost:9001' and let me know if that helps?

@martenson

This comment has been minimized.

Copy link
Member

commented Dec 7, 2016

@jmchilton shouldn't be in use; after the change to localhost it works

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Dec 7, 2016

Is 127.0.0.1 not a thing on Macs? I don't get it. @natefoo should this be localhost instead?

@martenson

This comment has been minimized.

Copy link
Member

commented Dec 7, 2016

127.0.0.1 is a thing, I use it (even with Galaxy) normally

jmchilton added some commits Aug 22, 2016

Bring Galaxy, reports, and shed run scripts in sync.
- They now uniformly handle server option processing.
- Reports and Tool Shed now respect many new and important options such as --skip_venv.
- Reports and Tool Shed now do extra checking around Python version that Galaxy does.

In addition to improving the reports and tool shed startup scripts and reducing cognative load with respect to option handling, this is an important pre-condition to us being able us to switch on uwsgi by default for all three services simultaneously without a bunch of copy and pasting.
Centralize launching server for reports and tool shed.
Initial outline of allowing launch via uwsgi.
Scripts to manage configuration infrastructure.
This has been rebased several times and includes a variety of fixes (some thanks to martenson).
Update tool shed to use newer configuration schema.
 - Implement kwalify schema to validate/define the schema.
 - Replace ini configuration with YAML (completely backward compatible).
 - Rebuild a sample configuration from the schema (config/tool_shed.yml.sample).
Update Galaxy Reports to use newer configuration schema.
 - Implement kwalify schema to validate/define the schema.
 - Replace ini configuration with YAML (completely backward compatible).
 - Rebuild a sample configuration from the schema (config/reports.yml.sample).
Add reports documentation to ``doc/``.
- Automatically generate RST docs from config schema.
- Describe older functionality as well as new YAML-based configuration.
- Replace https://wiki.galaxyproject.org/Admin/UsageReports.
Add required dependency on uwsgi.
Required for the new out-of-the-box configuration of tool shed and reports.

@jmchilton jmchilton force-pushed the jmchilton:web_config_overhaul_0 branch from 1f9f5b7 to 98ca8ce Aug 10, 2017

@jmchilton

This comment has been minimized.

Copy link
Member Author

commented Aug 10, 2017

Rebased... again...

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 10, 2017

restarted the tests, now everything is green
ping @natefoo and @erasche for review

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 11, 2017

@jmchilton the only thing I have found so far is a not so nice warning if you only have sample yml files.

make tool-shed-config-lint   
if [ -f .venv/bin/activate ]; then . .venv/bin/activate; fi; python lib/galaxy/webapps/config_manage.py lint tool_shed
WARNING: Path [None] not a file, using sample.

I think this can be improved in a later PR and we should get this in asap. Will merge after dinner if no one beats me to it.

uwsgi-rebuild-validation: ## rebuild uwsgi_config.yml kwalify schema against latest uwsgi master.
$(CONFIG_MANAGE) build_uwsgi_yaml

tool-shed-config-validate: ## validate tool shed YAML configuration file

This comment has been minimized.

Copy link
@erasche

erasche Aug 12, 2017

Member

since there are a number of lint / validation could we have a target for all of these? e.g. config-validate: uwsgi-rebuild-vlaidation toolshed-config-validate ...

This comment has been minimized.

Copy link
@jmchilton

jmchilton Aug 12, 2017

Author Member

uwsgi-rebuild-validation isn't something I'd expect deployers to run that is something we build and ship to them. I guess . a config validate that checked Galaxy, Reports, and Tool Shed configs if they are present would be cool though - I'd hope not too many people have tool shed configurations though 😓.

Configuration Options
----------------------------

.. include:: reports_options.rst

This comment has been minimized.

Copy link
@erasche

erasche Aug 12, 2017

Member

quite like this :D

This comment has been minimized.

Copy link
@jmchilton

jmchilton Aug 12, 2017

Author Member

Want to help me get this done for Galaxy for this release 😄?

@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 12, 2017

I talked to @erasche and his comments are not critical but nice to have. Would be great if this can be added in follow-up PR.
Tracking issue here: #4419

@bgruening bgruening merged commit 331851b into galaxyproject:dev Aug 12, 2017

5 checks passed

api test Build finished. 280 tests run, 0 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 153 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 37 tests run, 0 skipped, 0 failed.
Details
toolshed test Build finished. 579 tests run, 0 skipped, 0 failed.
Details
@bgruening

This comment has been minimized.

Copy link
Member

commented Aug 12, 2017

Thanks @jmchilton - this enables some nice additions in the future!

@martenson

This comment has been minimized.

Copy link
Member

commented Aug 12, 2017

Glad this got finally merged, thanks @jmchilton @bgruening @natefoo @erasche 🥇

@natefoo natefoo referenced this pull request Aug 22, 2017

Merged

Run Galaxy fully under uWSGI, including job handlers #4475

14 of 14 tasks complete
@scholtalbers

This comment has been minimized.

Copy link
Contributor

commented on config/reports.yml.sample in 102566d Nov 3, 2017

Where does one specify the prefix which used to be following in the ini section?

# Define the proxy-prefix filter.
[filter:proxy-prefix]
use = egg:PasteDeploy#prefix
prefix = /reports

This comment has been minimized.

Copy link
Member Author

replied Nov 3, 2017

Ping @natefoo - do you recall what we decided here?

This comment has been minimized.

Copy link
Member

replied Nov 9, 2017

Yeah, uWSGI can do this. I had a working example but I can't find that now, but I believe mount is what you want. We can add an example to the sample config once we have it working.

@jmchilton jmchilton referenced this pull request Dec 1, 2017

Merged

Convert options file to YAML and use uWSGI by default #5105

11 of 11 tasks complete
fi
export TOOL_SHED_CONFIG_FILE
fi

python ./scripts/paster.py serve $TOOL_SHED_CONFIG_FILE --pid-file=tool_shed_webapp.pid --log-file=tool_shed_webapp.log $args
find_server $TOOL_SHED_CONFIG_FILE
$run_server $server_args

This comment has been minimized.

Copy link
@nsoranzo

nsoranzo Jun 1, 2018

Member

@martenson @jmchilton After these changes we are setting but not using any more the $args variable, is this breaking the TS bootstrap?

This comment has been minimized.

Copy link
@jmchilton

jmchilton Jun 1, 2018

Author Member

I doubt I tested this so I don't know, no one has complained though 😅. I wouldn't worry too much about that functionality unless someone really intends to use it. I just double checked a fresh Planemo build and its bootstrapped toolshed is working - but it is probably called in a fairly custom way (xref https://github.com/galaxyproject/ansible-galaxy-toolshed/blob/d1355719f431f9228952d57c6c73b47394dba07f/tasks/database.yml#L29).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.