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

Add FastQE #103

Merged
merged 12 commits into from
Dec 4, 2023
Merged

Add FastQE #103

merged 12 commits into from
Dec 4, 2023

Conversation

michael-harper
Copy link
Contributor

Adding FastQE for onboarding documentation purposes

@MattWellie
Copy link
Contributor

Is there a reason for doing a micromamba installation in a naked OS image instead of starting with a python base image and installing with PIP?

Likewise is there a reason for google cloud sdk to be installed?

Both of these are bad CPG docker habits rather than good practice. Ideally you want a single tool installed in the simplest way possible.

@michael-harper
Copy link
Contributor Author

Is there a reason for doing a micromamba installation in a naked OS image instead of starting with a python base image and installing with PIP?

Likewise is there a reason for google cloud sdk to be installed?

Both of these are bad CPG docker habits rather than good practice. Ideally you want a single tool installed in the simplest way possible.

FastQE is going to need access to the files on GCS and so will need google cloud sdk (is there another way to approach this???). I tried not using micromamba and just using the google cloud sdk docker image but that was still around 1.3Gb I believe. The above solution manages to install fastqe and google cloud sdk straight-forwardly and is only marginally bigger at 1.4Gb.

Copy link
Contributor

@MattWellie MattWellie left a comment

Choose a reason for hiding this comment

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

I feel strongly that this image should be a tool-only image, with a minimal installation of a single tool. Currently this is modelled on other CPG Dockerfiles which don't follow good platform-independent practice.

@cassimons
Copy link
Contributor

Hey Michael, just piping in here that given this image is going to be the first image that new starters interact with I think we want to make sure it follows current best practice even if the practical difference in this case is limited.

FastQE is going to need access to the files on GCS and so will need google cloud SDK

Are you reading directly from buckets, or are you intending to use standard Batch file localisation? If so, you should not need the google libraries (and is probably the better method to use as a demonstration).

@cassimons cassimons removed their request for review October 31, 2023 21:00
Copy link
Contributor

@MattWellie MattWellie left a comment

Choose a reason for hiding this comment

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

LGTMatt

@michael-harper michael-harper merged commit 892f7a4 into main Dec 4, 2023
1 check passed
@michael-harper michael-harper deleted the add_FastQE branch December 4, 2023 02:44
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.

4 participants