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

CI: Upload test stats as artifacts, improve output of "List Docker images" #36970

Merged
merged 13 commits into from
Mar 25, 2024

Conversation

mkoeppe
Copy link
Contributor

@mkoeppe mkoeppe commented Dec 26, 2023

In a follow-up:

  • We could store the stats in a persistent database

📝 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 mkoeppe self-assigned this Dec 26, 2023
@mkoeppe mkoeppe changed the title CI: Upload test stats as artifacts CI: Upload test stats as artifacts, improve output of "List Docker images" Dec 27, 2023
Copy link

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

# ... and at the beginning of the next build stage,
# we check the status and exit with an error status.
CHECK_STATUS_THEN='STATUS=$(cat STATUS 2>/dev/null); case "$STATUS" in ""|0) ;; *) exit $STATUS;; esac; '
esac
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity, how does this trick avoid the problem of "With buildkit we cannot retrieve failed builds."?

How is "failed build" different from "exit" with nonzero status?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Out of curiosity, how does this trick avoid the problem of "With buildkit we cannot retrieve failed builds."?

By having the next build stage fail at the beginning, instead of failing the current one.

Copy link
Collaborator

Choose a reason for hiding this comment

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

so that the previous stage remains in the container and the current one discarded...

OK.

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 23, 2024

Otherwise lgtm as far as I can understand.

Is this change fully tested (in your fork)? Do you need to test again after 2 months?

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 24, 2024

Is this change fully tested (in your fork)?

Yes

Do you need to test again after 2 months?

Well, I've started another run now: https://github.com/mkoeppe/sage/actions/runs/8033793985

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 25, 2024

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

LGTM.

@kwankyu
Copy link
Collaborator

kwankyu commented Feb 25, 2024

OK. Let's see it in the battle field.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Feb 25, 2024

Thank you!

@vbraun vbraun merged commit 3abc045 into sagemath:develop Mar 25, 2024
33 of 44 checks passed
@mkoeppe mkoeppe added this to the sage-10.4 milestone Mar 25, 2024
vbraun pushed a commit to vbraun/sage that referenced this pull request Apr 25, 2024
…plex to 7, papilo to 2.2, pyscipopt to 5, onetbb to 2021.11

    
<!-- ^ Please provide a concise and informative title. -->
<!-- ^ Don't put issue numbers in the title, do this in the PR
description below. -->
<!-- ^ For example, instead of "Fixes sagemath#12345" use "Introduce new method
to calculate 1 + 2". -->
<!-- v Describe your changes below in detail. -->
<!-- v Why is this change required? What problem does it solve? -->
<!-- v If this PR resolves an open issue, please link to it here. For
example, "Fixes sagemath#12345". -->

https://www.scipopt.org/doc-9.0.0/html/RN90.php

- [x] scipopt/SCIP-SDP#12
- [ ] scipopt/soplex#35
- [x] needs pyscipopt update

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->

- [x] 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 accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on. For example,
-->
<!-- - sagemath#12345: short description why this is a dependency -->
<!-- - sagemath#34567: ... -->

- Depends on sagemath#37237
- Depends on sagemath#37351
- Depends on sagemath#36970
- Depends on sagemath#37392
    
URL: sagemath#37494
Reported by: Matthias Köppe
Reviewer(s): Kwankyu Lee
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