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

adds SeqFu #1047

Merged
merged 25 commits into from
Oct 4, 2024
Merged

adds SeqFu #1047

merged 25 commits into from
Oct 4, 2024

Conversation

taylorpaisie
Copy link
Contributor

Pull Request (PR) checklist:

  • [ X] Include a description of what is in this pull request in this message.
  • [ X] The dockerfile successfully builds to a test target for the user creating the PR. (i.e. docker build --tag samtools:1.15test --target test docker-builds/samtools/1.15 )
  • [ ]X Directory structure as name of the tool in lower case with special characters removed with a subdirectory of the version number (i.e. spades/3.12.0/Dockerfile)
    • (optional) All test files are located in same directory as the Dockerfile (i.e. shigatyper/2.0.1/test.sh)
  • [ X] Create a simple container-specific README.md in the same directory as the Dockerfile (i.e. spades/3.12.0/README.md)
    • If this README is longer than 30 lines, there is an explanation as to why more detail was needed
  • Dockerfile includes the recommended LABELS
  • [ X] Main README.md has been updated to include the tool and/or version of the dockerfile(s) in this PR
  • [ X] Program_Licenses.md contains the tool(s) used in this PR and has been updated for any missing

Comment on lines 1 to 18

##### ------------------------------------------------------------------------------------------------ #####
##### This caller workflow tests, builds, and pushes the image to Docker Hub and Quay using the most #####
##### recent version of Freyja and downloading the most recent variant information. #####
##### It takes no manual input. #####
##### ------------------------------------------------------------------------------------------------ #####

name: Update Freyja

on:
workflow_dispatch:
schedule:
- cron: '30 7 * * *'

run-name: Updating Freyja

jobs:
update:
Copy link
Contributor

Choose a reason for hiding this comment

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

I think file was included by accident

rm v${SEQFU_VER}.tar.gz && \
cd seqfu2-${SEQFU_VER} && \
make && \
make test VERBOSE=1 || (echo "Tests failed, but continuing build"; exit 0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you know why the tests fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh oops, i can take that out, just part of my debugging of the dockerfile, doesn't fail in the container now.


# Copy the necessary files from the builder
COPY --from=builder /usr/local/bin /usr/local/bin
COPY --from=builder /root/seqfu2-${SEQFU_VER}/data/tests /root/seqfu2-${SEQFU_VER}/data/tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you think it's worthwhile to include the test files in the final image? It might be helpful to just have these copied into the test stage for testing if they aren't needed during run time.

Comment on lines 67 to 71

# Verify installation
RUN ls -l /usr/local/bin && \
seqfu --version && seqfu --help

Copy link
Contributor

Choose a reason for hiding this comment

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

This is looking great!

If you're looking for assistance with creating tests, seqtk uses some fastq files from this repository in its tests

I imagine for seqfu it'd be someting like

RUN wget -q https://github.com/StaPH-B/docker-builds/raw/master/tests/SARS-CoV-2/SRR13957123_1.fastq.gz && \
    wget -q https://github.com/StaPH-B/docker-builds/raw/master/tests/SARS-CoV-2/SRR13957123_2.fastq.gz && \
seqfu count -f SRR13957123_1.fastq.gz -r SRR13957123_2.fastq.gz

SeqFu also looks like it comes with its own tests and files which may be useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, so i originally had all the test files in the container but you mentioned it wasn't necessary, so maybe i'll just put in the seqfu test fastq files you suggested in the comment for the seqfu test in the dockerfile. Also thank you for your help and suggestions!!!

@erinyoung
Copy link
Contributor

My apologies for my delayed response.

I think this looks great!

I did make some changes, though:

  • I moved your labels to the 'app' stage (labels don't transfer between stages and they get lost)
  • I added a CMD line to your 'app' stage (the CMD line is the default command that is run in a container, and we generally set that as the help message)
  • I removed some tools from the apt-get layer of the 'app' stage.
  • I added nim's version to the builder stage for "easy" updating later
  • I added in the dev's mini.sh tests - which all seem to work except test-check.sh, so I've dropped it (To me, this looks like a dev thing and not an installation issue)
  Testing module: check 
------------------------------------------------------------ 
OK: Checked single end exit status (expected 0, got 0)
OK: Checked single end OK (expected 1, got 1)
OK: Checked single end libtype (expected SE, got SE)
OK: Checked single end sequence count (expected 7, got 7)
OK: Checked BAD single end exit status with --deep (expected 1, got 1)
OK: Counts checking BAD single end exit status with --deep (expected 0, got 1)
OK: Checked BAD paired end exit status with --deep (expected 2, got 2)
OK: Counts checking BAD paired end exit status with --deep (expected 0, got 0)
OK: Checked BAD single end exit status without --deep (expected 1, got 1)
OK: Counts checking BAD single end exit status without --deep (expected 0, got 1)
OK: Checked paired end (expected 1, got 1)
OK: Checked paired end exit status (expected 0, got 0)
OK: Checked paired end libtype (expected PE, got PE)
OK: Checked single end sequence count (2xSE) (expected 14, got 14)
OK: Checked INVALID paired end (expected 0, got 0)
OK: Checked INVALID paired end exit status (got 1)
OK: Checked INVALID paired end libtype (expected PE, got PE)
OK: Checked INVALID paired end sequence count (2xSE) (expected -, got -)
OK: Checked INVALID directory (/test/test/../data//primers) exit status (expected > 0, got 2)
FAIL: Checked INVALID directory line count (got 14 expected 13)
OK      SE      /test/data/primers/16S_flash.extendedFrags.fastq        4306    1989406 0
OK      SE      /test/data/primers/16S_flash.extendedFrags.fastq.gz     4306    1989406 0
OK      SE      /test/data/primers/16S_merge.fq.gz      6137    2596981 0
OK      SE      /test/data/primers/16S_vsearch_merge.fq.gz      3935    1818111 0
ERR     SE      /test/data/primers/artificial.fq.gz     -       -       2       Invalid character in sequence: < > in R2.REV+.middle;
OK      SE      /test/data/primers/its-merge.fq.gz      7299    1504898 0
OK      SE      /test/data/primers/se.fq.gz     234     70434   0
OK      SE      /test/data/primers/small.fq     4       360     0
OK      PE      /test/data/primers/16S_R1.fq.gz 12274   3694474 0
OK      PE      /test/data/primers/16Snano_R1.fq.gz     468     140868  0
ERR     PE      /test/data/primers/art_R1.fq.gz 7       -       4       R2=Invalid character in sequence: < > in R2.REV+.middle;First sequence names do not match (R1.startFOR+, R2.startREV+);Last sequence names do not match (R1.FOR1+.start-middle, );
OK      PE      /test/data/primers/its_R1.fq.gz 16000   3387804 0
OK      PE      /test/data/primers/itsfilt_R1.fq.gz     15618   3272396 0
OK      PE      /test/data/primers/pico_R1.fq.gz        24      7224    0
total 10M
-rw-rw-r-- 1 root root 869K Dec 19  2023 16S_R1.fq.gz
-rw-rw-r-- 1 root root 986K Dec 19  2023 16S_R2.fq.gz
-rw-rw-r-- 1 root root 4.1M Dec 19  2023 16S_flash.extendedFrags.fastq
-rw-rw-r-- 1 root root 606K Dec 19  2023 16S_flash.extendedFrags.fastq.gz
-rw-rw-r-- 1 root root  256 Dec 19  2023 16S_flash.hist
-rw-rw-r-- 1 root root 1.4K Dec 19  2023 16S_flash.histogram
-rw-rw-r-- 1 root root 1.1M Dec 19  2023 16S_merge.fq.gz
-rw-rw-r-- 1 root root 416K Dec 19  2023 16S_vsearch_merge.fq.gz
-rw-rw-r-- 1 root root  33K Dec 19  2023 16Snano_R1.fq.gz
-rw-rw-r-- 1 root root  38K Dec 19  2023 16Snano_R2.fq.gz
-rw-rw-r-- 1 root root  233 Dec 19  2023 art_R1.fq.gz
-rw-rw-r-- 1 root root  236 Dec 19  2023 art_R2.fq.gz
-rw-rw-r-- 1 root root  299 Dec 19  2023 artificial.fa.gz
-rw-rw-r-- 1 root root  333 Dec 19  2023 artificial.fq.gz
-rw-rw-r-- 1 root root 273K Dec 19  2023 its-merge.fq.gz
-rw-rw-r-- 1 root root 355K Dec 19  2023 its_R1.fq.gz
-rw-rw-r-- 1 root root 500K Dec 19  2023 its_R2.fq.gz
-rw-rw-r-- 1 root root 328K Dec 19  2023 itsfilt_R1.fq.gz
-rw-rw-r-- 1 root root 440K Dec 19  2023 itsfilt_R2.fq.gz
-rw-rw-r-- 1 root root 2.1K Dec 19  2023 pico_R1.fq.gz
-rw-rw-r-- 1 root root 2.3K Dec 19  2023 pico_R2.fq.gz
-rw-rw-r-- 1 root root  33K Dec 19  2023 se.fq.gz
-rw-rw-r-- 1 root root 1011 Dec 19  2023 small.fq
OK: Checked INVALID directory line error count (got 2 expected 2)
OK: Checked valid directory SEQFU_TMPDIR_nevlh5 exit status (expected 0, got 0)
OK: Checked valid directory line count SEQFU_TMPDIR_nevlh5/out (got 1 expected 1)
OK: Checked valid directory line error count (got 0 expected 0)
FAIL: Finished with 23 passed, 1 failed

@erinyoung
Copy link
Contributor

@taylorpaisie , if you're okay with my changes, we can deploy this image to dockerhub and quay

@taylorpaisie
Copy link
Contributor Author

@erinyoung looks good to me! Thanks for all your advice and help, it's really helping me get better and learn more about building docker containers!

@erinyoung
Copy link
Contributor

Perfect!

I'm going to

  1. merge this PR
  2. deploy a 'seqfu' image to staphb's dockerhub and quay repositories
  3. use both tags of '1.20.3' and 'latest'

@erinyoung erinyoung merged commit f0017b6 into StaPH-B:master Oct 4, 2024
2 checks passed
@erinyoung
Copy link
Contributor

Thank you for putting this together!

You can check the status of the deploy here : https://github.com/StaPH-B/docker-builds/actions/runs/11184872785

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants