search

sul-dlss / sdr-client

SDR API client (in Ruby)
https://sul-dlss.github.io/sdr-api/
Other
0 stars 1 forks source link

Remove mime type from sdr-client #56

Closed mjgiarlo closed 4 years ago

mjgiarlo commented 4 years ago

Connects to sul-dlss/sdr-api#109

Why was this change made?

This ability is moving into sdr-api and is longer expected to be set upstream of the API.

Was the documentation (README, wiki) updated?

no

mjgiarlo commented 4 years ago

@jcoyne :speech_balloon:

Can we keep this for now? I wonder if it might be useful in the case that the automatic mime_type detection is faulty.

Any MIME-related sniffing that we can do upstream of sdr-client, can't we also do the same within sdr-api?

I'm wondering what the affordance is here vs. the potential confusion of suggesting that our expectation is that MIME types are provided upstream of the client.

jcoyne commented 4 years ago

@mjgiarlo I'm thinking that if an api consumer knows that this is "mime_type:application/bizaro" we should let them provide it. However if nothing is provided it'll fill in the value that ActionStorage is generating. application/bizaro might be preferable to application/octet-stream or whatever default they would otherwise get.

But I'll admit this is a speculative use case so I'm not feeling too strongly about it.

mjgiarlo commented 4 years ago

@jcoyne :speech_balloon:

@mjgiarlo I'm thinking that if an api consumer knows that this is "mime_type:application/bizaro" we should let them provide it. However if nothing is provided it'll fill in the value that ActionStorage is generating.

I am open to this, but also I want to ask whether this is a use case we know we have or a "just in case" change. I want to be certain this isn't YAGNI.

mjgiarlo commented 4 years ago

Let's hold off on merging this PR until @jcoyne and I have had a chance to chat on Friday, 1/31.

mjgiarlo commented 4 years ago

Spoke with @jcoyne and we decided to go forward with the approach in #58. For the time being, sdr-api will require upstream clients to generate MIME types and pass them along; sdr-api will not generate MIME types itself.

Why? It's fairly easy, for now, to do this upstream compared with the complexity of extending ActiveStorage in sdr-api which does not provide clean seams for wiring up MIME type generation.

It's true that ActiveStorage::Blob exposes methods that do automate MIME type generation, but none of these are invoked by the AS controllers we have pulled into sdr-api. It would take monkey-patching to achieve this, and that feels like the wrong move at this time.

Closing PR.