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

Add extension point for user preferences #3383

Merged
merged 44 commits into from Aug 8, 2017

Conversation

@bgruening
Copy link
Member

commented Jan 4, 2017

This pull request will implement an optional extension point to extend the user-preferences in a dynamic, admin-controlled way.

The admin defines in a yaml files allowed settings (name, type ...). A user can enter these settings and it will be saved in the user-object as external user preferences. These settings will become available inside the User object and with this accessible everywhere else.

Use cases:

Password Parameter Implementation (#3121)

A tool needs a password or any user-specific information, like @erasche Apollo stuff. Such a tool just need to assume the variable that it needs as environment variable ($MY_APOLLO_SERVER). An admin uses the dynamic_job_destinations to write a small function to map an arbitrary user-preference to this environment variable.

  • no tool syntax changes needed
  • no information is leaked during tool installation
  • central point to store and secure sensible data
  • a user only needs to enter once a configuration and reuse them whenever it is needed (workflow etc ...)

Webhooks can use personalized data

We have a overlay-search in development and it would be great if plugins can collect user-preferences, like the n in give me the n most used tools.
An other plugin we have developed is a simple RSS reader that shows interesting papers or science news after tool submission. To make this configurable we need such an extension point.

Bjoern's Blue Sky Idea

I would like to run a pulsar VM in our OpenStack cloud and register this specific VM in my account so that a few of my jobs can be calculated in my personal VM ... enabling me to bring my own compute resources.

I promise to write some documentation about the connection to dynamic_job_destinations and how you can abuse this very flexible idea if I get a green light.

@anuprulez

This comment has been minimized.

Copy link
Collaborator

commented on b3d7a02 Aug 12, 2016

@bgruening
user.preferences["dynamic_user_preferences"] = plugin_data is not working for me... is it for you?

This comment has been minimized.

Copy link
Owner Author

replied Aug 12, 2016

I can save something into the DB, but the structure is not like it should be :(

This comment has been minimized.

Copy link
Collaborator

replied Aug 12, 2016

user.preferences["dynamic_user_preferences"] = json.dumps( plugin_data ) works well for me with correct json and I have also changed that jQuery selector...shall I commit these changes ?

@galaxybot galaxybot added the triage label Jan 4, 2017

@galaxybot galaxybot added this to the 17.01 milestone Jan 4, 2017

@bgruening bgruening changed the title RFC: add extension point for user preferences Add extension point for user preferences Aug 7, 2017

@bgruening

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2017

I guess this is ready for final review.

@jmchilton

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

Looks great to me - thanks a bunch @bgruening !

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

Is it correct that the static JS files have changes but not the files in client/ ?

@bgruening

This comment has been minimized.

Copy link
Member Author

commented Aug 7, 2017

@mvdbeek this or probably due to different nodejs versions. I can try to get rid of this if it hurts.

@mvdbeek

This comment has been minimized.

Copy link
Member

commented Aug 7, 2017

If you haven't touched any JS directly (guessing from the files included in this PR), there should be no changes to the static/ files ... or am I totally wrong ? The important thing is that we don't lose changes in client/ of which there were some in this PR, but now they're gone (i.e 93d97d8). Maybe a bad merge ?

@bgruening

This comment has been minimized.

Copy link
Member Author

commented Aug 8, 2017

@mvdbeek I think what you are seeing there is our first version, adding a complete new category to the user-preference menu. Later one we just decided to extend the already existing one to not clutter the main menu, if I recall correctly. Looking back before the rebase: https://github.com/galaxyproject/galaxy/compare/dev...bgruening:5a86e3c5ca09932a67957afe44d9444dff8474f0?expand=1 are also no changes to client.

I think the changed static content is due to different node versions that build slightly different artifacts. I will try to get rid of those.

@@ -250,6 +252,47 @@ def anon_user_api_value( self, trans ):
'nice_total_disk_usage': util.nice_size( usage ),
'quota_percent': percent}

def _get_extra_user_preferences(self, trans):
"""
Reads the file user_preferences_extra_conf.xml to display

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Aug 8, 2017

Member

It's user_preferences_extra_conf.yml, right ?

This comment has been minimized.

Copy link
@bgruening

bgruening Aug 8, 2017

Author Member

Jupp, thanks for the catch.

@@ -0,0 +1,41 @@
preferences:
# the key you can refer to
apollo_url_01:

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Aug 8, 2017

Member

These are available by default, when it's not quite apparent why the user can enter apollo and openstack information. Should we maybe comment the first 2 sections ?

This comment has been minimized.

Copy link
@bgruening

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Aug 8, 2017

Member

Yes, that would be fine as well.

This comment has been minimized.

Copy link
@bgruening

bgruening Aug 8, 2017

Author Member

Fixed! Thanks!

type: text
# by defaul all inputs are required
required: True
- name: appollo_text

This comment has been minimized.

Copy link
@abretaud

abretaud Aug 8, 2017

Contributor

Just a little typo there: apollo_text
Cool to see progress on this!

if input['name'] == keys[1] and input['required']:
raise MessageException("Please fill the required field")
extra_user_pref_data[ item ] = payload[ item ]
user.preferences[ "extra_user_preferences" ] = json.dumps( extra_user_pref_data )

This comment has been minimized.

Copy link
@mvdbeek

mvdbeek Aug 8, 2017

Member

This is just nitpicking, but should we perhaps rename extra_user_preferences to static_user_preferences or admin_defined_user_preferences, to differentiate this from dynamic_user_preferences or tool_defined_user_preferences that we may want to implement in the future ?

@mvdbeek

mvdbeek approved these changes Aug 8, 2017

@erasche erasche merged commit 196a2bb into galaxyproject:dev Aug 8, 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. 150 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
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.