Skip to content

feature/ Add encryptionScheme=C4GH when calling htsget-rs#632

Merged
KarlG-nbis merged 3 commits intomainfrom
feature/htsget_add_encryption_scheme
Feb 11, 2026
Merged

feature/ Add encryptionScheme=C4GH when calling htsget-rs#632
KarlG-nbis merged 3 commits intomainfrom
feature/htsget_add_encryption_scheme

Conversation

@KarlG-nbis
Copy link
Copy Markdown
Contributor

Related issue(s) and PR(s)
This PR closes #631 .

Description
Adds encryptionScheme=C4GH when calling htsget-rs

@KarlG-nbis KarlG-nbis requested a review from a team as a code owner December 10, 2025 12:40
@KarlG-nbis KarlG-nbis force-pushed the feature/htsget_add_encryption_scheme branch from 597b4ba to fa74127 Compare December 10, 2025 12:41
Comment thread htsget/htsget.go Outdated
Copy link
Copy Markdown
Contributor

@zeidlitz zeidlitz left a comment

Choose a reason for hiding this comment

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

LGTM

@KarlG-nbis
Copy link
Copy Markdown
Contributor Author

KarlG-nbis commented Dec 16, 2025

Might pend another change as encountered another issue with c4gh keys and waiting for solution after discussion with Marko at: https://nbisweden.slack.com/archives/C063YCMEP6W/p1764231464294689

@KarlG-nbis KarlG-nbis force-pushed the feature/htsget_add_encryption_scheme branch from ddf404b to 7da55f8 Compare February 4, 2026 12:36
@KarlG-nbis
Copy link
Copy Markdown
Contributor Author

PR is now ready for review again after update in the htsget-rs application which requires the Htsget-Context-Public-Key

Copy link
Copy Markdown
Contributor

@nanjiangshu nanjiangshu left a comment

Choose a reason for hiding this comment

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

Looks good to me!

However, the way the URL is concatenated like this

url := htsgetHost + "/reads/" + datasetID + "/" + fileName + "?encryptionScheme=C4GH

is not ideal, probably better to use path.Join

But this is used at multiple places in the sda-cli code base, we can probably fix this in a refactoring issue.

@KarlG-nbis KarlG-nbis added this pull request to the merge queue Feb 11, 2026
Merged via the queue into main with commit d0050f2 Feb 11, 2026
6 checks passed
@KarlG-nbis KarlG-nbis deleted the feature/htsget_add_encryption_scheme branch February 11, 2026 15:02
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.

[htsget] Add encryptionScheme=C4GH when calling htsget-rs

4 participants