Skip to content
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

src/doc/en/developer/portability_testing.rst: Update after migration #35108

Merged
merged 5 commits into from May 22, 2023

Conversation

mkoeppe
Copy link
Member

@mkoeppe mkoeppe commented Feb 13, 2023

…to GitHub (no more sagetrac-mirror)

📚 Description

We update the documentation on portability testing.

📝 Checklist

  • I have made sure that the title is self-explanatory and the description concisely explains the PR.
  • I have linked an issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

Test running at: https://github.com/mkoeppe/sage/actions/runs/4911075567/jobs/8768884045
(the "standard", "minimal" etc. jobs without "-pre" should now work).

@codecov-commenter
Copy link

Codecov Report

Base: 88.60% // Head: 88.58% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (b2e7d1f) compared to base (3a044ad).
Patch has no changes to coverable lines.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #35108      +/-   ##
===========================================
- Coverage    88.60%   88.58%   -0.02%     
===========================================
  Files         2140     2140              
  Lines       396961   396961              
===========================================
- Hits        351714   351640      -74     
- Misses       45247    45321      +74     
Impacted Files Coverage Δ
src/sage/groups/affine_gps/euclidean_group.py 88.88% <0.00%> (-11.12%) ⬇️
...e/combinat/cluster_algebra_quiver/mutation_type.py 49.88% <0.00%> (-3.92%) ⬇️
src/sage/categories/super_modules_with_basis.py 96.29% <0.00%> (-3.71%) ⬇️
src/sage/groups/affine_gps/affine_group.py 96.62% <0.00%> (-2.25%) ⬇️
...rc/sage/categories/super_lie_conformal_algebras.py 95.00% <0.00%> (-1.67%) ⬇️
...onformal_algebras/lie_conformal_algebra_element.py 95.52% <0.00%> (-1.50%) ⬇️
src/sage/coding/channel.py 97.10% <0.00%> (-0.73%) ⬇️
...sage/geometry/hyperbolic_space/hyperbolic_model.py 88.95% <0.00%> (-0.62%) ⬇️
src/sage/quadratic_forms/ternary_qf.py 66.73% <0.00%> (-0.59%) ⬇️
src/sage/graphs/tutte_polynomial.py 93.57% <0.00%> (-0.46%) ⬇️
... and 17 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mkoeppe mkoeppe force-pushed the document_github_token_permissions branch from b2e7d1f to d45ec29 Compare May 8, 2023 03:42
@mkoeppe mkoeppe requested a review from tobiasdiez May 8, 2023 03:47
@mkoeppe mkoeppe self-assigned this May 8, 2023
generates one tarball. "Annotations" highlight certain top-level
errors or warnings issued during the build.

In addition to these automatic runs in our main repository, all Sage
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think its a good idea to advice people to run the workflows in their own forks. This a old workaround from the time where there was no easy way to run github workflows via trac. Instead I would propose to add labels to run the workflows, similarly how the conda workflow works.

Copy link
Member Author

Choose a reason for hiding this comment

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

No, that's exactly what we want -- so that such tests do not clog the project's Actions runners.

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we have enough parallel workflows now that we have a github team account? If a person only has a free github account, then it takes quite a bit of time until the complete matrix is finished since only a couple of them run in parallel - it also blocks other workflows in other projects. (Also not everyone wants to have 3000 published "packages": https://github.com/mkoeppe?tab=packages)

Copy link
Member Author

Choose a reason for hiding this comment

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

We have 60 runners, and the ci-linux workflow, running 24 jobs in parallel, takes a few days. I think that's too much for unleashing it on the project's runner pool.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think 24 jobs each taking a couple of hours (why would they take 'a few days'? its only compiling + running tests) should be okay with 60 runners. If not, we can remove some of the legacy systems and older python versions. I'm also a bit confused since here #35403 (comment) you denied that 'we might be hitting limits of what we have available on GitHub Actions.'

Either way, we cannot expect people to activate actions in their fork. I also don't know of any other open source project that would do this.

Copy link
Member Author

Choose a reason for hiding this comment

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

you denied that 'we might be hitting limits of what we have available on GitHub Actions.'

Context. We are not hitting limits when only every release tag triggers this heavy-weight workflow. But we would be in trouble if 3 of those would be run in parallel; it would stall all other types of workflows.

Copy link
Member Author

Choose a reason for hiding this comment

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

Either way, we cannot expect people to activate actions in their fork

And nobody has to do it unless they want to.

Copy link
Member Author

Choose a reason for hiding this comment

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

why would they take 'a few days'?

Just look at it: https://github.com/sagemath/sage/actions/workflows/ci-linux.yml

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not convinced that its a good idea to put this in the docs. If someone wants to run it in their fork, she is welcome to do this - but I don't think we should document and support this.

In total it might take a few days, but each workflow takes only a couple of hours and thus only blocks a runner for a few hours. Moreover, due to the staged setup not all parallel runers are consumed at the same time (e.g. currently there are only 5), so that other workflows could be run in between.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's already in the docs, and I put it there for an important purpose: Namely to empower developers to learn about portability work, and that includes running Actions by themselves, not just seeing them run by the magic central repository.

@mkoeppe
Copy link
Member Author

mkoeppe commented May 8, 2023

By the way, in #35380 I'm preparing a lighter-weight workflow that is suitable for casual runs in the main repo.

@github-actions
Copy link

github-actions bot commented May 8, 2023

Documentation preview for this PR is ready! 🎉
Built with commit: c49fc13

@mkoeppe
Copy link
Member Author

mkoeppe commented May 9, 2023

So let's get this in please.

Copy link
Member

@dimpase dimpase left a comment

Choose a reason for hiding this comment

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

lgtm

@mkoeppe
Copy link
Member Author

mkoeppe commented May 12, 2023

Thanks!

@vbraun vbraun merged commit 8b24c20 into sagemath:develop May 22, 2023
8 of 9 checks passed
@mkoeppe mkoeppe added this to the sage-10.1 milestone May 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants