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

Allow manually specifying media type. #233

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

adammcmaster
Copy link
Contributor

Closes #210

@eatyourgreens
Copy link

This looks good to me, but I don’t have a lot of Python experience. It will definitely solve a problem for generating LSST light curves from JSON strings.

@lcjohnso lcjohnso self-requested a review March 10, 2023 19:00
Copy link
Member

@lcjohnso lcjohnso left a comment

Choose a reason for hiding this comment

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

I'm not comfortable with an unchecked manual override of the MIME type. I feel this might create as many issues (as result of incorrect MIME type driving blob and media entry creation) as it solves.

Override would be acceptable if MIME type is validated, perhaps against a list of known & supported MIME types. That list might also be useful as part of a manual extension-to-MIME mapping solution that could replace current difficult-to-install libmagic MIME-typing solution.

@eatyourgreens
Copy link

Somewhat related to that, I’ve made a start on models to validate JSON files in JS. The models here will error if you pass them an invalid JSON-encoded string.
https://github.com/zooniverse/front-end-monorepo/tree/master/packages/lib-classifier/src/store/JSONData

@eatyourgreens
Copy link

eatyourgreens commented Mar 13, 2023

I've got a slightly different use case than the original issue here, so maybe this should be a new issue: my subjects are dynamically generated strings, not files on disk. At the moment, I'm generating the strings (in Python), writing them to files on disk, then looping back through those file paths and uploading them with subject.add_location(file_path).

This is really slow and inefficient. Since I already have each subject's contents stored in a string, it would be really useful if I could use those strings without having to write them to disk, then read them back again. At the moment, I can work around that by doing this:

dynamic_subject = '<?xml version="1.0" encoding="UTF-8" standalone="no"?><!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd"><svg width="391" height="391" viewBox="-70.5 -70.5 391 391" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><rect fill="#fff" stroke="#000" x="-70" y="-70" width="390" height="390"/><g opacity="0.8"><rect x="25" y="25" width="200" height="200" fill="lime" stroke-width="4" stroke="pink" /><circle cx="125" cy="125" r="75" fill="orange" /><polyline points="50,150 50,200 200,200 200,100" stroke="red" stroke-width="4" fill="none" /><line x1="50" y1="50" x2="200" y2="200" stroke="blue" stroke-width="4" /></g></svg>'
subject.locations.append('image/xml+svg')
subject._media_files.append(dynamic_subject)
subject.modified_attributes.add('locations')

where dynamic_subject in my script is a string returned from a function that builds the subject.

In my case I've got a new subject with two locations and two functions, one for each location. One function returns a JSON-encoded string, uploaded as application/json, and the second function returns a binary stream, uploaded as image/png.

@adammcmaster
Copy link
Contributor Author

Override would be acceptable if MIME type is validated, perhaps against a list of known & supported MIME types. That list might also be useful as part of a manual extension-to-MIME mapping solution that could replace current difficult-to-install libmagic MIME-typing solution.

I believe the API does validate the MIME type against an allowed list, so it will fail if an invalid type is given. Replacing libmagic with something easier is a good idea either way, though. File extensions might not always work, though, since you can pass an open file object instead of a path.

@eatyourgreens It might be worth seeing if StringIO would work better for your use case (I'm not certain if it would actually be any easier though).

@eatyourgreens
Copy link

Thanks! I will take a look.

@eatyourgreens
Copy link

File extensions might not always work, though, since you can pass an open file object instead of a path.

This essentially describes my problem.

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.

Correct mime types for json subjects
3 participants