Conversation
| // A flattend struct should work here, but produces runtime errors: | ||
| // unable to parse query string: invalid type: string "1", expected u64 | ||
| //#[serde(flatten)] | ||
| //limits: JobLimits, |
There was a problem hiding this comment.
Does anyone (@ahl maybe?) know why this flatten doesn't seem to work correctly? This is used in a QueryParams a few lines down (in job_start). There's no compile-time error with the flattened struct, and the client code is identical. I'm not really sure at what level the bug is, though there is this suspicious todo in Dropshot.
There was a problem hiding this comment.
This is an intrinsic limitation of serde at the intersection of flatten (which is a little janky) and serializations (such as serde_urlencoded) whose format is not self-describing with regard to types.
The way flatten works is that full object is deserialized into a generic container--kind of like serde_json::Value. The locally defined properties are pulled out; the rest form a Deserializer that is passed (in this case) to JobLimits::deserialize. This is necessary because derive macros only have local context. The generated code for serde::Deserialize on JobStartParams can't inspect the structure of JobLimits so it needs to delegate that flattened deserialization of it at runtime.
So how is the value deserialized into this generic container I mentioned? When deserializing max_cpu, say, serde knows to look for an integer, but when deserializing into this container we don't know if we're looking for an integer or a string or whatever. Some formats--such as JSON--are (mostly) self-describing: there isn't ambiguity whether a particular value should be a string, an integer, or a boolean.
For dropshot query parameters, however, we the url encoding (via serde_urlencoded) and that format is not self describing! When we get a query string like max_cpu=10&wait=false we can't tell if 10 should be treated as a string or a number; we can't tell if false should be a string or a boolean. Without the format telling serde what the type is supposed to be, it just has to guess... and as you might have guessed, in this case it guesses wrong. This should explain the error: invalid type: string "1", expected u64 the "1" gets deserialized into a string and then JobLimits::deserialize fails to deserialize max_cpu from a string.
You can see this in this playground.
I don't know of a satisfying workaround here. I bumped into @davepacheco asking about it here ... from 2020!
There was a problem hiding this comment.
Thanks very much for the detailed explanation! I was able to implement a workaround in 65b8867 that uses flatten with a deserialize_with method; because limit values are homogeneous (all u64), this was reasonably simple and I didn't have to embed any field names.
This is #3 in the big Mythos findings list.
Limits for CPU time, memory (address space size), and file size are currently available. Please let me know if you'd like more, or if I got any wrong (esp. memory, there are a few options but
ASseemed like the best).For example, we can limit a long-running process to 10 seconds of CPU time:
Tested on Linux and illumos.