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: add flux-pmix, flux-pam builds to CI #5489

Merged
merged 6 commits into from
Oct 9, 2023

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Oct 6, 2023

This PR adds the flux-pmix and flux-pam builds to CI to ensure PRs do not break the build/test of these projects.
flux-pam is probably less useful, but it is so quick to build I threw it in there.

This also fixes the after_n_builds support for codecov which might hopefully reduce the extra reporting we get for coverage.

grondo added 3 commits October 5, 2023 18:42
Problem: Changes proposed in a flux-core PR could break flux-pmix
and this could go undetected for some time.

Add a build and test of flux-pmix to the flux-core github workflow
so that PRs that might break flux-pmix are discovered when proposed.
Problem: A change that breaks flux-pam could be proposed in a
flux-core PR and go undetected.

Add a build of flux-pam to the flux-core github workflow so these
issues are discovered immediately.
Problem: `after_n_builds: 2` is set for the codecov `comment`
section, but to be truly effective it may have to also be set in
`codecov.notify`.

Add an `after_n_builds: 2` setting in the `codecov.notify` section of
`codecov.yml` in hopes this will get the feature working properly.
Copy link
Member

@chu11 chu11 left a comment

Choose a reason for hiding this comment

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

seems reasonable to me!

Problem: codeql action is having trouble with ubuntu repos.

Try running apt-get update before installing deps.
grondo added 2 commits October 9, 2023 08:04
Problem: The readthedocs build is failing with

 ImportError: cannot import name 'environmentfilter' from 'jinja2'

As suggested on the internets, pin jinja2<3.1.0 to fix this issue.
Problem: The default flux-security version used in ci is v0.9.0,
but the latest is v0.10.0.

Update the default FLUX_SECURITY_VERSION in docker-run-check.sh to
v0.10.0.
@grondo
Copy link
Contributor Author

grondo commented Oct 9, 2023

I also fixed the readthedocs build by pinning jinja2<=3.10 (not sure why that suddenly started failing).

Additionally, I noticed that docker-run-check.sh was still defaulting flux-security to v0.9.0, so that has now been updated to use v0.10.0.

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

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

Nice thanks!

@codecov
Copy link

codecov bot commented Oct 9, 2023

Codecov Report

Merging #5489 (d3c1a6d) into master (10abbf4) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5489      +/-   ##
==========================================
- Coverage   83.64%   83.62%   -0.02%     
==========================================
  Files         484      484              
  Lines       81443    81443              
==========================================
- Hits        68120    68110      -10     
- Misses      13323    13333      +10     

see 7 files with indirect coverage changes

@grondo
Copy link
Contributor Author

grondo commented Oct 9, 2023

Thanks! Setting MWP...

@mergify mergify bot merged commit 7cb4155 into flux-framework:master Oct 9, 2023
32 checks passed
@grondo grondo deleted the ci-flux-pmix branch October 9, 2023 18:23
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