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 Updates #1668

Merged
merged 5 commits into from Feb 15, 2024
Merged

CI Updates #1668

merged 5 commits into from Feb 15, 2024

Conversation

bennibbelink
Copy link
Contributor

This fixes the CI permissions problems we're seeing (along with cymetric #195 and cycamore #583). Sorry that this hasn't gone as smoothly as possible, hopefully this is the last major refactor.

Changes are as follows:

  • build_test.yml does not push any images to the cyclus GHCR. Instead it uses a local docker registry on the GitHub runner. This way we don't experience permissions errors with PRs from forks. This adds a tiny bit of complexity that I wanted to avoid, but honestly I think the workflow might be more intuitive now than in previous iterations. I tried implementing this without a local registry service but docker automatically tries to pull metadata so I was seeing lots of permission errors.
    • we still use build-contexts to "redefine" the cyclus image being used in downstream testing but now it references the image in our local registry
  • publish_latest.yml is the only workflow that updates the layer cache. This way all workflows essentially pull the layers that build the dependency image in cyclus:latest. Most workflows pull from this cache
  • code_coverage.yml has its own cache since its dependencies are slightly different, but I've included the ignore-error option so that cache-to won't stop a workflow from passing in a PR
  • all workflows that publish images (publish_latest.yml and publish_release.yml) need to be updated from a branch on the upstream repository. This is similar to our previous strategy
  • in the cycamore and cymetric PRs I changed the cyclus_tag/cycamore_tag in the Dockerfile to latest. This is to eliminate some verbosity in the workflow files and ensure users are using the latest versions when building from the Dockerfile

Once again I've reference branches in cycamore and cymetric corresponding to their respective PRs with these changes

@bennibbelink bennibbelink mentioned this pull request Feb 14, 2024
@coveralls
Copy link
Collaborator

coveralls commented Feb 14, 2024

Pull Request Test Coverage Report for Build 7920887563

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 1329 unchanged lines in 45 files lost coverage.
  • Overall coverage decreased (-16.7%) to 32.6%

Files with Coverage Reduction New Missed Lines %
src/any.hpp 1 39.49%
src/bid_portfolio.h 1 59.09%
src/env.h 1 53.85%
src/exchange_solver.cc 1 37.68%
src/exchange_translator.h 1 53.03%
src/greedy_preconditioner.cc 1 50.29%
src/logger.cc 1 68.97%
src/material.cc 1 76.74%
src/query_backend.h 1 81.5%
src/recorder.cc 1 74.32%
Totals Coverage Status
Change from base Build 7027753240: -16.7%
Covered Lines: 52950
Relevant Lines: 128926

💛 - Coveralls

@bennibbelink
Copy link
Contributor Author

Something this PR also does is add a trigger for code_coverage.yml on push to main. This will update the coverage reference in coveralls on a push to main. This did not exist before which I think is why we still see a coverage drop from 49%, we never ran the coverage workflow after merge to update the coverage of main

@bennibbelink bennibbelink marked this pull request as ready for review February 14, 2024 17:18
Copy link
Member

@gonuke gonuke left a comment

Choose a reason for hiding this comment

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

Hopefully we are done for now?

@gonuke gonuke merged commit b2ea7eb into main Feb 15, 2024
16 of 17 checks passed
@bennibbelink
Copy link
Contributor Author

Hopefully 🤞

@bennibbelink bennibbelink deleted the ci-updates branch February 15, 2024 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants