Skip to content

Conversation

@bernt-matthias
Copy link
Contributor

@bernt-matthias bernt-matthias commented Feb 12, 2026

@wm75 @pvanheus One of the pangolin tests is failing in the weekly tests. Problem is that the download of up-to-date data fails (in conda) with:

        File "/tmp/pip-build-env-md227tqc/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 333, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=[])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File "/tmp/pip-build-env-md227tqc/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 301, in _get_build_requires
          self.run_setup()
        File "/tmp/pip-build-env-md227tqc/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 520, in run_setup
          super().run_setup(setup_script=setup_script)
        File "/tmp/pip-build-env-md227tqc/overlay/lib/python3.11/site-packages/setuptools/build_meta.py", line 317, in run_setup
          exec(code, locals())
        File "<string>", line 4, in <module>
      ModuleNotFoundError: No module named 'pkg_resources'
      [end of output]

In order to fix this we would need to patch pangolin to add --no-build-isolation to the pip call.

IMO it might be better to remove the download functionality completely

  • data should be in the data table (we have a data manager)
  • each job downloads at least 140M

What do you think? Would this be acceptable?

FOR CONTRIBUTOR:

  • I have read the CONTRIBUTING.md document and this tool is appropriate for the tools-iuc repo.
  • License permits unrestricted use (educational + commercial)
  • This PR adds a new tool or tool collection
  • This PR updates an existing tool or tool collection
  • This PR does something else (explain below)

There are two labels that allow to ignore specific (false positive) tool linter errors:

  • skip-version-check: Use it if only a subset of the tools has been updated in a suite.
  • skip-url-check: Use it if github CI sees 403 errors, but the URLs work.

@bernt-matthias bernt-matthias marked this pull request as draft February 12, 2026 17:55
@bernt-matthias bernt-matthias mentioned this pull request Feb 12, 2026
49 tasks
@wm75
Copy link
Member

wm75 commented Feb 12, 2026

No objections against removing the latest data download. We decided to include the functionality during the pandemic because we thought it was just too much burden for instance admins to keep up with then frequent upstream updates and we did not want users to get outdated lineage classification results because of that.
Now that these hectic times are over, the reproducibility aspect is more important.

Interestingly, this might be one of those rare cases where not bumping the wrapper version might be good.
The latest version is the only one (on eu at least that has the issue) so not bumping would allow us to have only fully functional versions. A minor thing though and I doubt many people would run an old version with the latest download option.

@wm75
Copy link
Member

wm75 commented Feb 12, 2026

Oh, and one more thing. Please update the pangolin-data dependency to 1.36, which was the latest when this version of pangolin got released. Something to consider for the auto-updates in the future.

<description>Phylogenetic Assignment of Outbreak Lineages</description>
<macros>
<token name="@TOOL_VERSION@">4.3.4</token>
<token name="@PANGOLIN_DATA_VERSION@">1.26</token>
Copy link
Member

Choose a reason for hiding this comment

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

Ah, this one here and the requirement got unlinked. So the current tool ships with 1.36, but reports 1.26 in the interface.

Copy link
Member

Choose a reason for hiding this comment

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

same for @CONSTELLATIONS_VERSION@, the autoupdate bot just seems to overwrite ("bump") these tokens in the requirements.

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.

2 participants