Skip to content

Add bio command line tool - #110

Merged
tsibley merged 1 commit into
mainfrom
add-bio
May 12, 2025
Merged

tsibley merged 1 commit into
mainfrom
add-bio

Conversation

@joverlee521

@joverlee521 joverlee521 commented Apr 29, 2025 •

Copy link
Copy Markdown
Contributor

Description of proposed changes

Based on #108 to work-around CI hanging issues

Update: bio recipe has been fixed in bioconda with bioconda/bioconda-recipes#55920. This PR is now only blocked on #105

I wanted to add bio to match docker-base since I found it on bioconda.

Checklist

  • Checks pass

@tsibley

tsibley commented Apr 30, 2025

Copy link
Copy Markdown
Contributor

However, it doesn't work when I tested locally, might need to look into the bioconda recipe.

The bioconda recipe is borked. And the PR which added it is an amazing example of what not to do: deleting failing tests one-by-one just to get CI to pass and then merging without review.

image

orz

@joverlee521

Copy link
Copy Markdown
Contributor Author

The bioconda recipe is borked.

Can you elaborate? I'm not familiar enough with conda recipes to see what's wrong with it 😅

@victorlin

Copy link
Copy Markdown
Member

The recipe tells Bioconda how to build the Conda package, and it looks like the package we're getting was not built correctly. I'd guess that something in the build config needs tweaking.

@joverlee521

Copy link
Copy Markdown
Contributor Author

Created an issue in bioconda-recipes: bioconda/bioconda-recipes#55914

@tsibley

tsibley commented May 5, 2025 •

Copy link
Copy Markdown
Contributor

Can you elaborate? I'm not familiar enough with conda recipes to see what's wrong with it 😅

@joverlee521 I would have, but I didn't look into how it's borked, but I did confirm the borkage. After its installed, for example, the Python libraries are not present in site-packages, although the installation metadata is.

@joverlee521

Copy link
Copy Markdown
Contributor Author

See my hacky fix in bioconda/bioconda-recipes#55920

Matching nextstrain/docker-base#248.
Pin minimum version to 1.8.0 for NDJSON support.
@tsibley
tsibley changed the base branch from pin-awscli to main May 12, 2025 17:00
@tsibley
tsibley marked this pull request as ready for review May 12, 2025 17:00

@victorlin victorlin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Didn't test (not easily testable on Hutch network), but looks good given bio was added by all 3 builds in @tsibley's manually triggered CI run:

name: bio
version: 1.8.0
build: pyhdfd78af_0

@tsibley
tsibley merged commit 3f5bd36 into main May 12, 2025
@tsibley
tsibley deleted the add-bio branch May 12, 2025 20:37
@joverlee521

Copy link
Copy Markdown
Contributor Author

Thanks for following up here @tsibley 🙏

@tsibley

tsibley commented May 13, 2025

Copy link
Copy Markdown
Contributor

Since I was mucking around in this repo anyway, I figured I'd take care of this too. :-)

20250512T203714Z is the first version with bio.

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.

3 participants