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

Enable conda ci for all PRs and remove experimental label #36373

Merged
merged 2 commits into from
Oct 8, 2023

Conversation

tobiasdiez
Copy link
Contributor

@tobiasdiez tobiasdiez commented Oct 1, 2023

Using conda to install all sage dependencies is pretty stable and has been long enough in "experimental mode". We remove here the experimental flag and in order to make sure that the workflow is not breaking enable the conda-ci on all PRs. This also provides a) faster feedback as the whole build+test takes only about 1.5h (which is less time then what only the "incremental build" currently takes) and b) tests that the code is working on all supported python versions. The last remaining failing tests under linux are temporarily disabled in #36372.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • 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 accordingly.

⌛ Dependencies

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 1, 2023

This also provides a) faster feedback as the whole build+test takes only about 1.5h (which is less time then what only the "incremental build" currently takes)

I don't think this comparison makes sense. The (incremental) Build & Test CI takes long currently because there is no current image on ghcr.io. It is building all the major package upgrades that have happened since 10.2.beta1.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 1, 2023

I'm in favor of running the conda ci more often; I agree that this could help with reducing bitrot.

It's probably fine to run it on every PR, but I think it should not launch all 3 Linux and 3 macOS runs immediately. This would seriously clog the macOS pipeline.

Perhaps start with one run first and only launch the others on success?

@tobiasdiez
Copy link
Contributor Author

Perhaps start with one run first and only launch the others on success?

So limit the number of parallel matrix jobs to say 2, and activate "fail-fast", or did you have something else in mind?

We can also disable the macos workflows for now since they are failing anyway.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 2, 2023

Perhaps start with one run first and only launch the others on success?

So limit the number of parallel matrix jobs to say 2, and activate "fail-fast", or did you have something else in mind?

Sure, that sounds fine.

We can also disable the macos workflows for now since they are failing anyway.

That would not help. As you already know, #35593 fixes the macos runs, so it could be merged simultaneously if not unforeseen obstacles arise.

@tobiasdiez
Copy link
Contributor Author

Perhaps start with one run first and only launch the others on success?

So limit the number of parallel matrix jobs to say 2, and activate "fail-fast", or did you have something else in mind?

Sure, that sounds fine.

Implemented.

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 2, 2023

Thanks, looking good.

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Documentation preview for this PR (built with commit e7d0a44; changes) is ready! 🎉

@tobiasdiez
Copy link
Contributor Author

Thanks for the positive review!

@vbraun vbraun merged commit 16c78ca into sagemath:develop Oct 8, 2023
17 of 24 checks passed
@mkoeppe mkoeppe added this to the sage-10.2 milestone Oct 8, 2023
@tobiasdiez tobiasdiez deleted the conda_non_experimental branch October 10, 2023 01:09
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.

3 participants