Fixing data_source tools and incrementing tool profile

[wm75]

Still discovering a few quirks with data_source and data_source_async tools after #17422

• currently, these tools have to set check_values="false" in their <inputs> tag.
  Forgetting to do this leads to hard to debug problems, because the default is "true", while this can never make sense for any data_source tool. The proposed solution is to overwrite the setting for the tool class.

• currently, translation of input params is requested in several places in the code base, i.e., at the controllers level here:
  

  galaxy/lib/galaxy/webapps/galaxy/controllers/tool_runner.py

  Lines 86 to 87 in 2a9c8d0

  	if tool.input_translator:
  	tool.input_translator.translate(params)

  and here:
  

  galaxy/lib/galaxy/webapps/galaxy/controllers/async.py

  Lines 83 to 84 in 2a9c8d0

  	if tool.input_translator:
  	tool.input_translator.translate(params)

  
  and at the tool level, here:
  

  galaxy/lib/galaxy/tools/__init__.py

  Lines 1834 to 1835 in 2a9c8d0

  	if self.input_translator:
  	self.input_translator.translate(expanded_incoming)

  and here:
  

  galaxy/lib/galaxy/tools/__init__.py

  Lines 2502 to 2503 in 2a9c8d0

  	if self.input_translator:
  	self.input_translator.translate(params)

  The controller-level translations are required, but repeated translations at the tool level can break data_source tools.
  In particular, URL append operations are broken because the same params get added to the URL twice.
  That's why I'd like to disable the tool-level translations, but I'm worried that this could have side-effects, and really would like feedback first. We could play it safe and disable extra translations only for data_source tools with sth like:
  if self.input_translator and not isinstance(self, DataSourceTool): just in case input translation has some exotic undocumented use outside data_source tools?

- [ ] I need help with an additional bug with this usage pattern where an ampersand is used to join GET request parameters. Unfortunately, that char gets sanitized by Galaxy (to an X by default), but I fail to understand where that happens

@mvdbeek maybe you can help me with the open points? 🙏

How to test the changes?

(Select all options that apply)

• Instructions for manual testing are as follows:
  1. run considerable subset (or all) data source tools on Main

License

• I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

[wm75]

Ah, the sanitization issue is solved by using a tool-wide <options sanitize="False" />. Now I know why existing data_source tools include that :-)

That leaves only the multiple translator calls.

[wm75]

Ok, so once more I'm happy with my changes and think they are ready for reviewing.

[davelopez]

Thank you @wm75! Looks good to me!

[martenson]

this commit is breaking any tool that relies on undeclared params, which I suspect could be a pattern that is actually used? I am a bit wary about doing that.

[wm75]

I am a bit wary about doing that.

Was so too, that's why I didn't touch this in my prior PR. However, out of the built-in data source tools only the UCSC ones seem to work currently and I checked and updated these to still work after the change.
Any other data source tools not shipping with Galaxy would be easy to fix.

[martenson]

Even if easy to fix, breaking tools is something Galaxy works hard to avoid. We have tool profiles to mitigate that, maybe we could consider applying it here.

[wm75]

Even if easy to fix, breaking tools is something Galaxy works hard to avoid. We have tool profiles to mitigate that, maybe we could consider applying it here.

That's a very good idea, thanks! Implemented it in the latest commit.

[wm75]

The one failing test appears to be unrelated again :-)

[jdavcs]

Looks good to me too! OK to merge?

[martenson]

Thanks @wm75 !

I believe this PR should be manually tested on Main for the release. I added the item to the PR description.