-
Notifications
You must be signed in to change notification settings - Fork 51
Tool config data immutability flag resolves #1054 #1059
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
Conversation
prioux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some minor changes.
| </div> | ||
| <% end %> | ||
|
|
||
| <% t.edit_cell(:inputs_readonly, :header => "Read only input files", :show_width => 2) do |f| %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Within the web page, let's not use the 'inputs_readonly' phrasing, it's super ugly. Let's show this as 'Does not modify its inputs files'
| <% t.edit_cell(:inputs_readonly, :header => "Read only input files", :show_width => 2) do |f| %> | ||
| <%= f.check_box :inputs_readonly %> | ||
| <div class="wide_field_explanation"> | ||
| Are you sure that input files are not mutated by any task? It will allow execution with data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next text:
Check this if the tool is known <strong>not to modify its input files</strong>. This will allow a user to launch the tool on files that are not marked as group-writable in the file manager.
| @@ -0,0 +1,5 @@ | |||
| class AddInputsReadonlyToToolConfig < ActiveRecord::Migration[5.0] | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add license header. Check other migration for examples.
|
I just realized that the Boutiques integrator is not checking that new flag. Please adjust There is a complexity there. At line 103 the template sets a constant value in the CLASS based on what is found in the descriptor. We can't adjust that, because at the class level we have no way to find out the configuration at run time that is in the tool config. But at line 120 is where the run-time code looks it up, so basically we need the code to check the property in in the tool config there. It will be tru if it's true EITHER in the descriptor, or in the tool config. |
prioux
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more change requested, see discussion panel.
|
Please fix the tests. Dozens of them fail now when trying to invoke inputs_readonly on nil. |
No description provided.