Skip to content

rm internal tools#1914

Open
gsaudade99 wants to merge 3 commits into
usegalaxy-eu:masterfrom
gsaudade99:patch/simplify_internal_tools
Open

rm internal tools#1914
gsaudade99 wants to merge 3 commits into
usegalaxy-eu:masterfrom
gsaudade99:patch/simplify_internal_tools

Conversation

@gsaudade99

Copy link
Copy Markdown
Contributor

Now we have the conditions in tool_defautls I think we can remove this

@gsaudade99 gsaudade99 self-assigned this Feb 26, 2026

_basic_internal_tool:
asbtract: true
rules:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

we don't have that in our internal destination definition, do we?

There is no reject: pulsar ?

@gsaudade99 gsaudade99 Feb 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think we added a condition to not add a pulsar tag to the expression tools under the id remote_resources

@domgz domgz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hmm, I am not sure whether to proceed or not with this one.

There are internal tools, including __DATA_FETCH__, that do not define a tool_type.

On the other hand, I would expect such cases to be covered by tool.requires_galaxy_python_environment == True. But this 10-year old code doesn't look very trustworthy to me...

That should not be very hard to check if we set up the debug handler. Afaik Galaxy loads a registry of all installed tools in memory, so it would be a matter of inserting a breakpoint, looking into the registry a and making sure tool.requires_galaxy_python_environment == True for all these tools.

Ofc this could also be achieved without the debug handler, for example changing the code so that when a handler finishes loading it writes this information to a file.

@bgruening

Copy link
Copy Markdown
Member

xref: https://github.com/usegalaxy-eu/galaxy/blob/697a55524bdfd94275f8bf0308846eaf6d017a30/lib/galaxy/tools/__init__.py#L266

@gsaudade99

Copy link
Copy Markdown
Contributor Author

@domgz

domgz commented Feb 26, 2026

Copy link
Copy Markdown
Contributor

xref: https://github.com/usegalaxy-eu/galaxy/blob/697a55524bdfd94275f8bf0308846eaf6d017a30/lib/galaxy/tools/__init__.py#L266

Settles it no @kysrpex ?

I can only say that I am not sure, I would have to check more carefully to be sure.

@gsaudade99

gsaudade99 commented Mar 3, 2026

Copy link
Copy Markdown
Contributor Author

So I have analyzed the piece of code and cross referenced with the tools that we have mentioned in tools.yml:

    @property
    def requires_galaxy_python_environment(self):
      ...
      else:
              unversioned_legacy_tool = self.old_id in GALAXY_LIB_TOOLS_UNVERSIONED
              versioned_legacy_tool = self.old_id in GALAXY_LIB_TOOLS_VERSIONED
              legacy_tool = unversioned_legacy_tool or (
                  versioned_legacy_tool and self.old_id and self.version_object < GALAXY_LIB_TOOLS_VERSIONED[self.old_id]
              )
              return legacy_tool

This means that any tool that is unversioned tool in the list GALAXY_LIB_TOOLS_UNVERSIONED will return true:

For our tools this is true in:

  • export_remote
  • DATA_FETCH
  • upload1

For the other three tools, the tooltype is explicit in the tool datasouce:

  • param_value_from_file - expression tool
  • ucsc_table_direct1 - data_source
  • toolshed.g2.bx.psu.edu/repos/iuc/compose_text_param/compose_text_param/. - expression

This leaves the edge case of other tools that match the regex __.*__ that are not in the list above mentioned and are not type annotated in the tool source.

Where can I find more tools that would match that regex? If we can know this is safe to merge this PR.

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.

3 participants