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

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

Merged
merged 57 commits into from Jan 18, 2018

Conversation

Projects
None yet
7 participants
@jmchilton
Member

jmchilton commented Nov 30, 2017

What's In It?

The default sample configuration for Galaxy is now config/galaxy.yml.sample instead of config/galaxy.ini.sample. If a galaxy.yml configuration is found - run.sh will start with uWSGI instead of paste by default. This means all new Galaxy instances will target uWSGI by default.

Beyond just swapping the configuration format, this PR provides important new tooling and documentation for swapping.

  • make config-convert-dry-run - will demonstrate how to convert an existing config/galaxy.ini into a config/galaxy.yml.
  • make config-convert - will actually perform this configuration.
  • config-validate - will validate a galaxy.yml file for formal correctness using pykwalify.
  • config-lint - will lint a galaxy.yml file and warn of various potential issues.
  • The admin section of the documentation now features RST documentation for the Galaxy configuration options.

Backward Compatibility

Existing galaxy.ini should continue to work just fine and will still cause Galaxy to start a paster server when coming from run.sh - so this should be effectively completely backward compatible for existing admins.

Adding Configuration Options to Galaxy

The sample configuration file still exists, but the file lib/galaxy/webapps/galaxy/config_schema.yml containing the specification should now be considered the canonical documentation for a Galaxy configuration option. The sample YAML file and the RST documentation can be automatically re-generated from that source using the Make targets config-rebuild-sample and config-rebuild-rst respectively.

Conversion and Linting Galaxy Parameters

The following dict describes some of the basic handling of converting and linting Galaxy parameters - hopefully it makes a little sense (there are test cases if you want to dig in deeper).

OPTION_ACTIONS = {
    'use_beaker_session': _DeprecatedAndDroppedAction(),
    'session_type': _DeprecatedAndDroppedAction(),
    'session_data_dir': _DeprecatedAndDroppedAction(),
    'session_key': _DeprecatedAndDroppedAction(),
    'session_secret': _DeprecatedAndDroppedAction(),
    'paste.app_factory': _PasteAppFactoryAction(),
    'filter-with': _HandleFilterWithAction(),
    'debug': _ProductionUnsafe(True),
    'serve_xss_vulnerable_mimetypes': _ProductionUnsafe(True),
    'use_printdebug': _ProductionUnsafe(True),
    'use_interactive': _ProductionUnsafe(True),
    'id_secret': _ProductionUnsafe('USING THE DEFAULT IS NOT SECURE!'),
    'master_api_key': _ProductionUnsafe('changethis'),
    'external_service_type_config_file': _DeprecatedAndDroppedAction(),
    'external_service_type_path': _DeprecatedAndDroppedAction(),
    'enable_sequencer_communication': _DeprecatedAndDroppedAction(),
    'run_workflow_toolform_upgrade': _DeprecatedAndDroppedAction(),
    # Next 4 were from library search which is no longer available.
    'enable_lucene_library_search': _DeprecatedAndDroppedAction(),
    'fulltext_max_size': _DeprecatedAndDroppedAction(),
    'fulltext_noindex_filetypes': _DeprecatedAndDroppedAction(),
    'fulltext_url': _DeprecatedAndDroppedAction(),
    'enable_legacy_sample_tracking_api': _DeprecatedAction(),
    'enable_new_user_preferences': _DeprecatedAndDroppedAction(),
    'force_beta_workflow_scheduled_for_collections': _DeprecatedAction(),
    'force_beta_workflow_scheduled_min_steps': _DeprecatedAction(),
    'history_local_serial_workflow_scheduling': _ProductionPerformance(),
    'allow_library_path_paste': _RenameAction("allow_path_paste"),
    'trust_ipython_notebook_conversion': _RenameAction("trust_jupyter_notebook_conversion"),
    'enable_beta_tool_command_isolation': _DeprecatedAndDroppedAction(),
    'single_user': _ProductionUnsafe(True),
    'tool_submission_burst_threads': _ProductionPerformance(),
    'tool_submission_burst_at': _ProductionPerformance(),
    'toolform_upgrade': _DeprecatedAndDroppedAction(),
}

If other options should have custom conversion or special warning/info during linting let me know and I'd be happy to add it.

A Note on Reviewing this PR

I'd encourage review of this PR via commit - at least as it pertains to the options file. I've tried to automate the conversion of the ini sample file into a YAML configuration specification (via kwalify and the infrastructure setup in #3179). I've done this step-by-step so you can see the manual edits of the content versus the automated conversion steps in separate commits. I've also done this in such a way that git log --follow lib/galaxy/webapps/galaxy/config_schema.yml preserves the history of the config/galaxy.ini.sample.

This PR is broken into a few different chunks. The commits prefixed with config_manage.py are fixes to the infrastructure for YAML parsing adding in #3719. The commits prefixed with convert_yaml are the actual conversion process. And finally the commits prefixed with yaml_config is the generation of new artifact, Makefile targets, etc... for the new configuration specification.

TODO

  • Rework amqp option parsing to allow specification via YAML.
  • Document logging parsing to allow specification via YAML. @natefoo
  • Freeze 17.09 documentation and add links to it to dev documentation on pages that heavily mention Galaxy options. (xref #5120, d7a9108)
  • Fill out new RST page on Galaxy options - right now it is an index but it should have an introduction and probably some discussion of migrating. Is there a good place to start on the hub for generating material for this? @jmchilton
  • Improve list of unsafe, beta, and dropped configuration options to improve new configuration file linter. @jmchilton
  • Create a Github issue outlining further work - including allowing other configurations to be embedded in galaxy.yml, adding more annotations to config_schema.yml to improve documentation, and pre-processing the type information to simply config.py considerably. #5148
  • Update nginx configuration related proxy prefix handling (still references ini) - see nginx.md. @natefoo
  • Update apche configuration related proxy prefix handling (still references ini) - see apache.md. @natefoo
  • Update description of "servers" in jobs.md (still references ini). @natefoo
  • Re-write scaling.md to eliminate references to ini and reflect newer uWSGI by default. @natefoo
  • First first setup CI test - still broken.

There is a corresponding WIP Hub PR here (galaxyproject/galaxy-hub#345).

@jmchilton jmchilton added this to the 18.01 milestone Nov 30, 2017

```
There is also the option for tracking jobs in database that is required to be turned on for the account activation to be effective. By default it is off.
```
track_jobs_in_database = True
track_jobs_in_database true

This comment has been minimized.

@bgruening

bgruening Dec 1, 2017

Member

colon missing?

This comment has been minimized.

@jmchilton

jmchilton Dec 1, 2017

Member

Yup - I'll get it on a rebase. Thanks.

@jmchilton

This comment has been minimized.

Member

jmchilton commented Dec 1, 2017

@natefoo The first startup CI test is broken - any ideas?

executing: .venv/bin/uwsgi --module 'galaxy.webapps.galaxy.buildapp:uwsgi_app()' --virtualenv /home/travis/build/galaxyproject/galaxy/.venv --pythonpath lib --master --threads 4 --http localhost:8080 --static-map /static/style=/home/travis/build/galaxyproject/galaxy/static/style/blue --static-map /static=/home/travis/build/galaxyproject/galaxy/static --die-on-term --hook-master-start 'unix_signal:2 gracefully_kill_them_all' --hook-master-start 'unix_signal:15 gracefully_kill_them_all' --enable-threads --py-call-osafterfork  --daemonize2 galaxy.log --safe-pidfile galaxy.pid
/home/travis/build/galaxyproject/galaxy/.venv/bin/python2.7: unrecognized option '--safe-pidfile'
getopt_long() error
cat: galaxy.pid: No such file or directory
.ci/first_startup.sh: line 13: kill: `': not a pid or valid job spec
exit code:1, showing startup log:
cat: galaxy.log: No such file or directory
@natefoo

This comment has been minimized.

Member

natefoo commented Dec 4, 2017

Looks like it's documented but was never merged to uWSGI 2.0: unbit/uwsgi#1417. I'll open a PR for that, but in the meantime I guess we will need to do this with --pidfile, which unfortunately has some issues that --safe-pidfile was created to solve.

@natefoo

This comment has been minimized.

Member

natefoo commented Dec 4, 2017

2.0 backport PR'd upstream in unbit/uwsgi#1691, hopefully this will be in 2.0.16. In the meantime, I've switched out --safe-pidfile for --pidfile2. Without --safe-pidfile it means if you accidentally run sh run.sh --daemon twice, you can no longer stop the old one with sh run.sh --stop-daemon.

@natefoo

This comment has been minimized.

Member

natefoo commented Dec 4, 2017

I didn't necessarily mean for that last commit to go here but it's sorta become the "uWSGI post-merge fixes" PR. I can strip it if you'd prefer not to have it mixed in here though.

jmchilton added some commits Nov 30, 2017

config_manage.py - allow printing comment header on sample descriptio…
…n files.

Generated from the desc field in the app schema.
config_manage.py - fix sample file generation for large default values.
The various messages in the Galaxy config were being split across lines and generating invalid YAML.
config_manage.py - hack uWSGI section of sample file generation.
Because uWSGI seems really bad at parsing fairly simple, entirely valid YAML.
Fix ignoring ".sample" in newish path module.
Required to correctly detect the extension of sample configuration files.
config_manage.py - fix to generate YAML booleans in RST docs.
The new default - might as well use that representation.
yaml_config - Add RST docs for Galaxy options.
Automatically generated from the new specification.

natefoo added some commits Jan 12, 2018

Attempt to reduce duplication in the proxy documentation through the use
of includes, move the multi-config-file discussion to a common page.
@natefoo

This comment has been minimized.

Member

natefoo commented Jan 12, 2018

One last thing I want to fix before this is ready to be rebased: the default uWSGI options in the sample config should match the options that scripts/get_uwsgi_args.py generates on the command line.

('master', {
'desc': """Enable the master process manager. Disabled by default for maximum compatibility with CTRL+C, but should be enabled for use with --daemon and/or production deployments.""",
'default': 'false',
'type': 'str',

This comment has been minimized.

@natefoo

natefoo Jan 15, 2018

Member

@jmchilton if I set this to bool then the values in the sample are pythonic (e.g. True and False).

@natefoo

This comment has been minimized.

Member

natefoo commented Jan 15, 2018

Ok, ready for rebasing, @jmchilton.

@natefoo natefoo modified the milestones: 18.05, 18.01 Jan 17, 2018

@martenson martenson changed the title from [WIP] Convert options file to YAML and use uWSGI by default to Convert options file to YAML and use uWSGI by default Jan 17, 2018

@natefoo

This comment has been minimized.

Member

natefoo commented Jan 18, 2018

@jmchilton says I'm allowed to merge this but I feel a little funny doing it since half the commits are mine.

@natefoo

This comment has been minimized.

Member

natefoo commented Jan 18, 2018

How about this: everything I added look ok to you @jmchilton?

@martenson martenson merged commit 7659efc into galaxyproject:dev Jan 18, 2018

6 checks passed

api test Build finished. 343 tests run, 4 skipped, 0 failed.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
framework test Build finished. 168 tests run, 0 skipped, 0 failed.
Details
integration test Build finished. 67 tests run, 0 skipped, 0 failed.
Details
selenium test Build finished. 118 tests run, 2 skipped, 0 failed.
Details
toolshed test Build finished. 577 tests run, 0 skipped, 0 failed.
Details
@martenson

This comment has been minimized.

Member

martenson commented Jan 18, 2018

huge leap forward, thanks boyz!

@jmchilton

This comment has been minimized.

Member

jmchilton commented Jan 18, 2018

How about this: everything I added look ok to you @jmchilton?

The docs you added are so beautiful that I'm glad our project license doesn't prevent me from publishing a book of poetry that is just snippets of your wonderful words!

@martenson

This comment has been minimized.

Member

martenson commented Jan 18, 2018

Btw is there an alternative for run.sh --reload?

@natefoo

This comment has been minimized.

Member

natefoo commented Jan 18, 2018

@jmchilton hrm, this much flattery is not suspicious at all. ;D

@martenson --py-autoreload, but I bet it fails, because there is a bug with uwsgi-as-an-exension that I haven't yet fixed or (and it looks like I haven't created an issue for it) where it thinks its application name for re-execution is python (because it doesn't expect its executable to be a script).

You can work around that by doing a pip install --upgrade --no-binary uwsgi --no-cache-dir uwsgi to install from source.

@jmchilton

This comment has been minimized.

Member

jmchilton commented Jan 18, 2018

Might make sense to have --reload default to paste for now then.

@jxtx

This comment has been minimized.

Contributor

jxtx commented Jan 18, 2018

image

@natefoo natefoo referenced this pull request Jan 25, 2018

Closed

Replace paste#http with uwsgi #2393

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment