Support remote URLs for loading scripts #55
Conversation
|
Not sure what the best way to handle updating scripts (matching by name?). Would be nice to have it automatically creating new versions from the command-line too as it would make it possible for people to auto-update scripts using cron. |
|
The issue is that the storage backend will change the name of the file automatically on saving it. We'll need to add in a method on the storage class to detect this and handle it appropriately. I'm not sure at the moment what is the most robust way to catch updates as well other than something like (script_group, filename) |
|
I guess this only really applies to the command line interface though? When updating through the admin it's up to the person doing it to update the correct script. We just need a way to determine that automatically added scripts are newer versions of older scripts. I wonder if we can do something clever with the source (URL/folder) or the script filename (before import)? |
|
Unfortunately we won't have a script group when loading from the command line. I guess we could provide an option to supply one (probably makes sense) and then in the absence of once check the filename against all script groups. Since the slug is a unique feature perhaps creating the script slug from the filename would give us a solution? Not sure if we would want to copy this approach to the admin UI also? That way at least repeatedly importing from the command line would correctly create multiple versions rather than multiple scripts. |
|
You can already supply a script group on the command line via --group. However, for bulk loading it's useless since you can't divvy up scripts. How about a user prompt and a --no-input option like collectstatic? If there is a conflict via name -- ask the user to confirm if it's a new or updated script. If they already know the answer, --no-input will let them skip it all. |
|
Sounds good, I'll have a look at how to implement this based off collectstatic. |
This commit supports loading scripts from remote URLs and zip or gzip files. Archives are automatically de-compressed and all contained files imported. Folders are now walked recursively (as archives commonly have a first level folder) and all found scripts imported. Will need to handle the case of pre-existing scripts (by name?) to be able to support automatic updating without creating multiple script objects.
|
Rebased on current master. |
This catches the source script basename and passes it into add_wooey_script allowing for version updating from the commandline. Using this re-adding the same scripts over and over creates new versions (we will want to suppress this in future by using a hash of some kind).
|
Ah. Adding via the commandline leaves multiple versions with |
|
I don't know how unique would help us since it's just a boolean and not taking things like script, script_version as a combined key. My current approach is to just set the old one to non-default and the current to a new default. I think we could automate it with a pre_save hook and check if default_version is changing. |
|
You could just do Script.latest_version.default_version = False as well, since that will automatically get the current default_version. |
|
Ah forgot about latest_version. The commit just uses the |
| @@ -217,7 +222,7 @@ def add_wooey_script(script_version=None, script_path=None, group=None): | |||
| if isinstance(group, ScriptGroup): | |||
| group = group.group_name | |||
| if group is None: | |||
| group = 'Wooey Scripts' | |||
| group = 'Scripts' | |||
There was a problem hiding this comment.
Shouldn't this point to our config for the default group?
There was a problem hiding this comment.
Yes it should be WOOEY_DEFAULT_SCRIPT_GROUP, will fix.
|
The last bit to implement I guess is the confirmation check for new versions. I've also got some suggested changes for the name parsing (from the filename) to make things a bit more attractive. There is a standard Python package ...obviously it isn't correct for the fasta in the example (because the source file is all lowercase). But if you have a file named with |
|
Sorry, I meant doing a combined key with the |
|
👍 on the titlecase. What modelfield supports a combined key with unique=True. Are you talking about just adding an extra slug like field? I think it'd be simpler just to do it in the clean method of the model and raise an error from there. |
|
I was thinking of the I'll add the titlecase into this PR. It adds a dependency on the titlecase package (it's on PyPi) but I don't think that's a biggie. |
This automatically applies title case to scripts loaded from the command line. The resulting name is used for matching against existing entries in the database, so having a uniform method to generate this is important. Uses the titlecase library which applies standard transformations while leaving pre-capitalized words intact.
|
There is a weird error on the build, not sure what that is about (Django 1.6 only). |
|
It broke from this: https://travis-ci.org/wooey/Wooey/jobs/79476685 Something with using the wooey settings. Maybe fix it by adding the relevant setting to the test settings? |
|
Yeah looking at it I think it's the translation string on the default script group |
|
Removing lazy evaluation works but thinking about it it will break the dynamic translations on the front page so I used the non-lazy approach for the The tagline is still lazy evaluated (so it's dynamic for visitors browser settings). I removed the translation off "Wooey!" since it's a brand-name (of sorts) so we won't want it to change. If people specify the names themselves they're obviously free to override these. For the |
Evaluate Scripts non-lazy in the wooey_settings file to use the default system locale for this. Note that this will be overridden and ignored if this value is set specifically by the user. Translation has been removed from Wooey! as it's a brand-name. The site tag remains translated with lazy evaluation, so it can automatically match the visitors browser settings.
|
So (again) the last bit to implement is the confirmation check for new versions. I've been testing this in combination with the new clinto argparse getter and it seems to be working on anything I throw at it which is good. I'm going to take some time to go over the biocode script repo and update the file parameters so we can detect them correctly — then we'll have a good source for examples of "load lots of scripts quickly". |
|
👍 |
|
The database should be agnostic to the translations. It will be stored as whatever DEFAULT_SCRIPT_GROUP is set to inside the database (which will be English), and if a translation is provided for the display out, then it will be displayed. The only caveat obviously is if they change that default group on the database, then another will be created that is the default value again. However, I view this as a usage error and should be corrected by indicating they change the value in their settings as opposed to the database. |
|
To update on what I was saying 6(!) months ago. I hit a problem with the biocode script repo in that most of the scripts required the ability to import relative files (i.e. they weren't self contained but relied on utils/base scripts that were packaged with them). This comes back to out support for this sort of thing generally but is a separate issue to this one. I'll take a further look at the new version confirmation check bit (once I've figured out what I meant by that) and we should be good to merge. Surprised it is still able ;) |
|
Maybe we can have a new model that is Worker Settings that can hold additional environmental variables so users can add paths/etc. A simple model would be: Then the worker can just do os.environ[name] = value and such |
This commit supports loading scripts from remote URLs
and zip or gzip files. Archives are automatically de-compressed
and all contained files imported.
Folders are now walked recursively (as archives commonly have a
first level folder) and all found scripts imported.
Will need to handle the case of pre-existing scripts (by name?)
to be able to support automatic updating without creating
multiple script objects.