https://nf-co.re logo
Join Slack
Powered by
Vacation’s over, let’s start with something fun (g...
# request-review
m
Vacation’s over, let’s start with something fun (gatk4 bulk update) github.com/nf-core/modules/pull/12738
🤣 3
homer disappear 3
here we go again
😂 2
m
ok, let's have a look
could you do my template sync in exchange?
gatk4-main? why?
shared a GIF
Made some comments
But good job for such a big PR
m
Smth with the linting. Asked for feedback from bioconda core
@Maxime Garcia, how did you manage to fix the missing 
gzip
issue with singularity in the samtools PR?
m
I did what now?
Oh yeah, I remember
m
Most singularity tests are failing for the stubs because 
gzip
is missing for some reason
m
I used bgzip, not gzip
m
didn’t that break the linting? I think I tried that
m
Not if you follow linting
Check the PR
The trick is to walk a fine line between following what it asks and doing what you need
Channel your inner French
Embrace #C0A2RDVD9L7
m
Since this seems to be a larger issue, I’d actually like to fix it in a more sustainable way 🤔
seems to me the singularity base image over at seqera is just missing gzip 🤷🏻
m
Yeah, then fixing the linting tools?
Do we need gzip when we have bgzip?
m
no, but in the case of gatk, it’s an extra dependency either way
for samtools it makes sense to use bgzip, since it’s part of the toolkit
I don’t want to bloat the gatk image even more
m
Agreeing with you
m
To fix this at the core, I invoke the mighty @Phil Ewels, master of containers, ruler of OSS
TLDR, lately singularity tests with seqera containers are failing stub tests because 
gzip
is missing. I suggest this to be added to the base image
p
hello hello whatnow
🤣 1
No need for me to do anything: chuck in an issue / PR on the wave repo, it's public
I can advocate if needed
m
I can do a PR if you can point me to the right file.
p
or for different templates (miniconda etc)
But I think nf-core is specifying the pixi one now? off the top of my head?
m
no idea 🤷🏻
p
yeah so the link I posted above is the one that we care about
m
🤔 any idea which one is used in the webinterface?
p
that's just "whatever is default at the moment", which I think is currently the micromamba v2 one?
but note that adding stuff to these base images is a huge pain as it means we have to change the default base image which invalidates the cache for all new image requests
m
Per the suggestion of Paolo, maybe this can be fixed in #CE5LG7WMB github.com/nf-core/tools/pull/4449
p
Yeah that's what I was thinking earlier, but you specifically said Seqera Containers
m
container support is #CE5LG7WMB is still fairly new, so not my mental default yet 😅
p
Once we get further with #C09V4RAHV0X hopefully you won't need a mental default at all
Just specify some conda packages and leave the rest to the magic magic
m
you won’t need a mental default at all
I like to think I’m already mental by default 😄
😆 2
FYI, replaced the containers for GATK with ones built by #CE5LG7WMB, including the changes in github.com/nf-core/tools/pull/4449 and the issue seems to be fixed
🎉 2
m
NICE
m
Fixed the final issues, all tests should pass now. Feel free to give a final once over, otherwise I’ll merge once the CI is done
👀 1
m
just 1 comment
❤️ 1
feel free to merge in anyway, and we can fix in a separate PR
m
I’ll fix it in another PR, save some polar bears
party this 1
so we don’t have to re-test 70 modules
2 Views
Open in Slack
PreviousNext
Powered by