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

refactor: separate storage and axum crates #253

Merged
merged 9 commits into from
Aug 4, 2024

Conversation

mmalenic
Copy link
Member

@mmalenic mmalenic commented Aug 2, 2024

Changes

  • Move the storage module from htsget-search to it's own htsget-storage crate.
  • Move the data_server.rs component to it's own htsget-axum crate.
    • Implement a htsget-axum binary identical to htsget-actix, and recommend usage of htsget-axum instead of htsget-actix.
  • Bump noodles and Lambda dependencies.

Motivation

The htsget-search crate has had two functionalities from the get go, which was the actual htsget searching algorithm, and a storage interface to interact with files. This PR splits these into two separate crates, because in the future, further storage interfaces like Crypt4GH will be implemented, which will no longer fit well into a single crate.

This change also moves the Axum-based data_server to it's own crate, which doesn't really belong in htsget-search or htsget-storage. With this crate, there is a new htsget application using Axum. This is useful because other components of htsget-rs depend on Axum, like htsget-lambda, and even htsget-actix uses an Axum-based data block server. This means that Axum makes more sense to use within htsget-rs, and is the recommendation going forward.

The htsget-actix crate will still be maintained because it is relatively simple to do so, and for backwards compatibility.

Future work

Since htsget-axum is now it's own dedicated crate, other components should be migrated to Axum. This includes the Dockerfile, and htsget-lambda, which can use the Axum router directly. This would simplify htsget-lambda significantly, as custom request parsing would no longer be required.

Benchmarking between htsget-actix and htsget-axum should also be unified.

I've saved this for a future PR to avoid this one getting too large.

@mmalenic mmalenic self-assigned this Aug 2, 2024
@mmalenic mmalenic added the refactor changes related to code refactoring label Aug 2, 2024
Copy link
Member

@brainstorm brainstorm left a comment

Choose a reason for hiding this comment

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

Very nice refactor and thanks for the focus on the docs too, kudos!

@brainstorm brainstorm added this pull request to the merge queue Aug 4, 2024
Merged via the queue into main with commit 492bdb6 Aug 4, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor changes related to code refactoring
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants