Skip to content

feat: remove http based file ingest#108

Open
jennifer-bowser wants to merge 17 commits into
mainfrom
issue-103-remove-http-based-file-ingest
Open

feat: remove http based file ingest#108
jennifer-bowser wants to merge 17 commits into
mainfrom
issue-103-remove-http-based-file-ingest

Conversation

@jennifer-bowser

Copy link
Copy Markdown
Contributor

closes #103

@jennifer-bowser jennifer-bowser self-assigned this Jun 26, 2026
@jennifer-bowser jennifer-bowser added the priority:low Low priority label Jul 9, 2026
@jennifer-bowser
jennifer-bowser marked this pull request as ready for review July 9, 2026 17:51
@jennifer-bowser
jennifer-bowser requested a review from a team as a code owner July 9, 2026 17:51

@jsstevenson jsstevenson 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.

I know this is leftover code, but there are a few things that I think we should not carry over from the previous endpoint

  1. file size limit -- the current max is too low to really be usable anyway, but I don't think a CLI needs to enforce a file size limit on itself. Since this is non-public, there's no reason why we need to be defensive against ourselves.
  2. validate gzip -- pysam can also read non-gzipped VCFs just fine, I don't think we should be introducing additional validation beyond what it thinks it can handle (ditto for the filename).
  3. the _raise_vcf_ingestion_error function -- I think this might be an unnecessary additional layer, in several cases it is re-logging exceptions that have already been logged, and also makes the stack trace that prints to the console even longer.
    • with respect to point # 2 above -- pysam will raise a ValueError if you try to give it an unreadable file. I think we should be catching this, logging, and reraising it directly in the service function instead of doing a big catch block in the CLI

Comment thread src/anyvlm/cli.py Outdated
Comment thread src/anyvlm/cli.py Outdated
@jennifer-bowser
jennifer-bowser marked this pull request as draft July 20, 2026 16:21
@jennifer-bowser
jennifer-bowser marked this pull request as ready for review July 20, 2026 16:21

@jsstevenson jsstevenson 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.

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority:low Low priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove HTTP-based file ingest

2 participants