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

fix: add content type to metadata file #425

Merged
merged 7 commits into from
Jan 29, 2025
Merged

Conversation

kaloster
Copy link
Contributor

@kaloster kaloster commented Jan 23, 2025

Relates to chanzuckerberg/cryoet-data-portal#1390

Description

This one makes sure that ContentType is set only for metadata json files

Tested here and here.

@kaloster kaloster force-pushed the kaloster/content-type-open branch from daa57df to 90d78f6 Compare January 23, 2025 18:03
@kaloster kaloster requested review from jgadling and manasaV3 January 23, 2025 18:06
@kaloster kaloster requested a review from jgadling January 23, 2025 18:20
@jgadling
Copy link
Contributor

Is it practical to write some tests for this, to make sure that we set content type properly for json files, and also that content types for images aren't weird?

@kaloster kaloster force-pushed the kaloster/content-type-open branch from 319e75f to a440649 Compare January 24, 2025 21:09
Copy link
Contributor

@jgadling jgadling left a comment

Choose a reason for hiding this comment

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

Can you add a test to ensure that non-json files are written without a content-type: json header?

Ok to merge after that, thanks for the fixes!

@kaloster kaloster merged commit 273c39d into main Jan 29, 2025
6 checks passed
@kaloster kaloster deleted the kaloster/content-type-open branch January 29, 2025 16:54
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