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

rnaseq ARM Module updates #7101

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from
Draft

rnaseq ARM Module updates #7101

wants to merge 8 commits into from

Conversation

edmundmiller
Copy link
Contributor

@edmundmiller edmundmiller commented Nov 26, 2024

Updates all of the versions in the rnaseq modules to ensure they have ARM conda builds.

@edmundmiller edmundmiller self-assigned this Nov 26, 2024
@edmundmiller edmundmiller requested a review from ewels November 26, 2024 22:53
mashehu
mashehu previously approved these changes Nov 27, 2024
@@ -52,12 +52,12 @@ jobs:
# if: github.repository == 'nf-core/modules'
if: ${{ needs.generate-matrix.outputs.conda-matrix != '[]' }}
needs: generate-matrix
name: Build ${{ matrix.profile }} ${{ matrix.platform }} Container
name: 🌊 ${{ matrix.files }} ${{ matrix.profile }} ${{ matrix.platform }}
Copy link
Contributor

Choose a reason for hiding this comment

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

🙅🏻 makes it difficult for newcomers to understand what is happening

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I can agree with that.

I was trying to clean up the names because it's also hard for newcomers and me to figure out what's happening when the name is cut off 🙃

Open to suggestions 🤷🏻‍♂️

image

runs-on: ubuntu-latest
timeout-minutes: 60
strategy:
fail-fast: false
max-parallel: 4
# max-parallel: 2
Copy link
Contributor

Choose a reason for hiding this comment

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

why not 4? we have cores, so why not use them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I turned it off for the purpose of running them all quickly
  2. I don't know exactly how that will translate to AWS ec2 runners. A lot of these jobs will just ping wave and be done. So I didn't want to spin up 20 runners because it's getting ready for a lot of runs and we're going to need a lot of capacity, and then it's 20 of the same wave runs because it was GATK.

Copy link
Contributor

Choose a reason for hiding this comment

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

can we make this action fail gracefully for now, meaning that it exits with code 0 in any way? makes the launch of this less disruptive.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, we don't want all the CI to start failing. Maybe we could test locally for inspecting which containers are failing? and make this action always pass

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've been doing that; this was @ewels' ask because he'd like a list of what modules are failing 🤷🏻‍♂️

Let's do a Slack hook or do that in an alternative script if we still want the wave jobs to pass.

Copy link
Member

Choose a reason for hiding this comment

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

Are we adding successful builds to meta.yml? That'd be nice. Then we could just not add to meta.yml on fail but not trigger a CI failure for now.

A slack hook on build fail would be nice if easy 🤔

…ules

- Bump bracken version from 2.9 to 3.0
- Update samtools version from 1.16.1 to 1.21
- Upgrade star version from 2.7.10a to 2.7.11b in both rsem modules
@edmundmiller edmundmiller changed the title ci: Let wave fail ci: Let wave fail and test rnaseq arm Jan 20, 2025
@ewels
Copy link
Member

ewels commented Jan 20, 2025

Let's strip out all changes unrelated to the new Wave CI

@edmundmiller edmundmiller changed the title ci: Let wave fail and test rnaseq arm rnaseq ARM Module updates Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In progress
Development

Successfully merging this pull request may close these issues.

4 participants