yetanalytics / lrs

Protocols, specifications, and logic for building an xAPI Learning Record Store (LRS) in Clojure(Script).
https://www.yetanalytics.com/lrs
Apache License 2.0
4 stars 1 forks source link

Content-length and content-type are optional #69

Closed milt closed 2 years ago

milt commented 2 years ago

Update the content-length spec to reflect that it is nilable. 1.x says that the client SHOULD* send it. In 2.0 this should change to a MUST and the LRS should throw a 400 if no content-length is provided.

A client must either send a content-length or transfer-encoding: chunked header. If they do the latter, the content length will not be known when the inputstream is passed to the impl. Additionally, the content-type may be left out of some documents so we pass this through to the impl.

milt commented 2 years ago

I think for that one I'd prefer a default, how about binary/octet-stream

kelvinqian00 commented 2 years ago

That is what I am implementing on the lrsql side actually: https://github.com/yetanalytics/lrsql/blob/f4f8d2465e4535273584773684d3a162027852d1/src/main/lrsql/input/document.clj#L71

milt commented 2 years ago

Yeah, I'll just make it nilable and leave it up to the implementers then. Btw, I thing application/octet-stream might have executable connotations, not sure. binary/octet-stream is used in other parts of the spec. Really don't know which is best

kelvinqian00 commented 2 years ago

Actually application/octet-stream is what's used in parts of the xAPI spec (see here).

milt commented 2 years ago

Ah, so it is. Thrilling

kelvinqian00 commented 2 years ago

Ok currently testing the updated spec in lrsql - will need to update third down the line as well.

kelvinqian00 commented 2 years ago

This is what I'm talking about to be clear: https://github.com/yetanalytics/lrsql/blob/main/src/main/lrsql/spec/document.clj#L86

milt commented 2 years ago

If I change that, I'll have to change source (where it sends a document to the impl) and that will take longer

milt commented 2 years ago

but sure