Skip to content

Conversation

@user202729
Copy link
Contributor

@user202729 user202729 commented Nov 16, 2025

Temporary fix for #39244

📝 Checklist

  • The title is concise and informative.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation and checked the documentation preview.

⌛ Dependencies

@user202729 user202729 added the p: CI Fix merged before running CI tests label Nov 16, 2025
@user202729 user202729 marked this pull request as draft November 16, 2025 10:29
@user202729 user202729 changed the title Add internet doctest marker to various places Add internet doctest marker to various places using gap group atlas Nov 16, 2025
@user202729 user202729 removed the p: CI Fix merged before running CI tests label Nov 16, 2025
@antonio-rojas
Copy link
Contributor

Some distributions (at least Arch and Gentoo) include the atlasrep data with their gap packages and don't need internet access - this will unnecessarily skip these tests there unless they are run with optional=internet, which is not desirable because lots of internet tests are flaky.

A better fix would be to add a feature test for the availability of the atlasrep data, and use it in the relevant tests.

@user202729
Copy link
Contributor Author

user202729 commented Nov 16, 2025

  • looks like there's no such feature test yet.
  • where are the files (supposed to be) stored and how do I check for them? (too bad the server is down now so...)
  • wait, the user can customize where to store these files right?
  • actually it's probably possible to do the feature test by looking into the source code of the package itself and see how it check for file availability, then call these functions... not sure calling libgap function at feature-test time is a good idea though.

@antonio-rojas
Copy link
Contributor

antonio-rojas commented Nov 16, 2025

My idea is not to check for the files presence (which, as you said, can be in multiple places, even downloaded by gap itself in a previous run), but rather run one of these gap computations that need the data in the feature test, with a reasonable timeout (say 5s), and disable the feature if it times out.

@user202729
Copy link
Contributor Author

user202729 commented Nov 16, 2025

note on the failure at the later step: it's triggered by the (mere) import of sage.libs.gap.all_documented_functions. I can't reproduce it (the import on my machine raises an error), but it can be reproduced with libgap.eval('AtlasOfGroupRepresentationsInfo') or libgap.function_factory('AtlasOfGroupRepresentationsInfo')

@user202729 user202729 added the p: CI Fix merged before running CI tests label Nov 16, 2025
@user202729
Copy link
Contributor Author

server is back up.

@antonio-rojas do you know how Arch inject the data to the package? As a (slightly more reliable) workaround we may do that on the CI first.

@antonio-rojas
Copy link
Contributor

server is back up.

@antonio-rojas do you know how Arch inject the data to the package? As a (slightly more reliable) workaround we may do that on the CI first.

See https://gitlab.archlinux.org/archlinux/packaging/packages/gap/-/blob/4.15.1-1/PKGBUILD?ref_type=tags#L106

Unfortunately GAP will still try to contact the network by default (and hang if the website is down) even if the data is installed, so one needs to disable that option, see https://github.com/gentoo/gentoo/blob/master/dev-gap/atlasrep/files/atlasrep-2.1.7-no-remote-access.patch

@cxzhong
Copy link
Contributor

cxzhong commented Nov 18, 2025

Since the server is OK now, we do not need this in CI

@cxzhong cxzhong removed the p: CI Fix merged before running CI tests label Nov 18, 2025
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