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.

Sign up for GitHub

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

Jump to bottom

add gvcf file for cttsov2 and add content disposition header for s3 p… #345

Merged
merged 2 commits into from
Jul 11, 2024
Merged

add gvcf file for cttsov2 and add content disposition header for s3 p… #345

merged 2 commits into from
Jul 11, 2024

Conversation

raylrui
Copy link
Contributor

@raylrui raylrui commented Jul 10, 2024 •
edited
Loading

Fix:

  1. include .gvcf files in the analysis tables
  2. add 'Content-Disposition' header in 'preview' s3 presignurl api, and change default presignurl behaviour to download as attachment, this will couple PR in data-portal-apis and libumccr.aws.libs3.presign_s3_file to be able add 'content_disposition' params
  3. fix some other type issues

Apis PR: umccr/data-portal-apis#705
Libumccr PR: umccr/libumccr#28

Reference:

@raylrui raylrui self-assigned this Jul 10, 2024
This was linked to issues Jul 10, 2024
Analysis Results cttsov2 pipeline files download issue #342
Closed
Add .gvcf in the Analysis Results tables #340
Closed
@raylrui raylrui added this to the Release 2.2.2 milestone Jul 11, 2024
@raylrui raylrui marked this pull request as ready for review July 11, 2024 02:11
victorskl
victorskl approved these changes Jul 11, 2024
View reviewed changes
Copy link
Member

@victorskl victorskl left a comment

Choose a reason for hiding this comment

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

LGTM. Just one minor comment to attend.

src/containers/subjects/AnalysisResultTable/index.tsx Outdated
(r) => r.key.includes('cttsov2') && (r.key.endsWith('.vcf') || r.key.endsWith('.vcf.gz'))
(r) =>
r.key.includes('cttsov2') &&
(r.key.endsWith('.vcf') || r.key.endsWith('.gvcf') || r.key.endsWith('.vcf.gz'))
Copy link
Member

Choose a reason for hiding this comment

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

Can you also add .gvcf.gz here, pls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, yes

@raylrui
Copy link
Contributor Author

raylrui commented Jul 11, 2024

Merging to see

@raylrui raylrui merged commit 3ca67ed into dev Jul 11, 2024
1 check passed
@victorskl victorskl deleted the fix/add-content-disposition-for-s3-presign-api-request branch July 15, 2024 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@victorskl victorskl victorskl approved these changes

@williamputraintan williamputraintan Awaiting requested review from williamputraintan

Assignees

@raylrui raylrui

Labels
change request fix
Projects
None yet
Milestone
Release 2.2.2
Development

Successfully merging this pull request may close these issues.

Analysis Results cttsov2 pipeline files download issue Add .gvcf in the Analysis Results tables
2 participants
@raylrui @victorskl