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

Plugin installation #1909

Merged
merged 69 commits into from
Oct 4, 2016
Merged

Plugin installation #1909

merged 69 commits into from
Oct 4, 2016

Conversation

josenavas
Copy link
Contributor

@josenavas josenavas commented Jul 27, 2016

Branch that allows plugins to register themselves.

TODOs:

  • Modify the DB so plugins can be active/inactive
  • Create a parser for the files in the plugins dir to install/(de)activate plugins (Plugin installation parser #1923)
  • Add CLI to update plugins and modify webserver start to activate only available plugins.
  • Add REST api for plugins so they can register artifact types, file types/extensions and commands
  • Update relevant GUI so only active plugins/commands/types are visible

If you think that there is something missing in this TODO list, please add it. If you have a concern about this, please come and talk to me.

Fixes #1625

@josenavas
Copy link
Contributor Author

Note: tests here are failing because I changed the contents of the config file (added an entry). For fixing that I need to update the encoded config file for travis, but I'm not going to do that until this PR is ready to merge, so other PR can be done against master.

@@ -116,6 +116,8 @@ class ConfigurationManager(object):
The filepath to the portal styling config file
plugin_launcher : str
The script used to start the plugins
portal_dir : str
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be plugin_dir?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right!

@ElDeveloper
Copy link
Member

👍 just a few minor comments/questions.


# Warning raised if No files will be allowed to be uploaded
# Warning raised if no cookie_secret
with warnings.catch_warnings(record=True) as warns:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not really a comment - but some motivation on switching over to PY3

We can substitute this with assertWarns

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does assertWarns allow you to check the contents of the warning? We have been using npt.assert_warns in other places of the code, but here I decided to use the catch warnings approach so I can test that the contents of the warnings is what I expect.

Updating the phred_offset type to be choice
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.455% when pulling 1388858 on plugin-installation into 93486ff on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.465% when pulling 38f00e5 on plugin-installation into 93486ff on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.449% when pulling 95bf847 on plugin-installation into 93486ff on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.449% when pulling 326e369 on plugin-installation into d03c11b on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.451% when pulling 5704b2c on plugin-installation into d03c11b on master.

@josenavas josenavas changed the title WIP: Plugin installation Plugin installation Oct 4, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.4%) to 84.451% when pulling 88084c9 on plugin-installation into d03c11b on master.

@antgonza antgonza merged commit ef74ef2 into master Oct 4, 2016
@josenavas josenavas deleted the plugin-installation branch October 4, 2016 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants