Sled agent views/params shuffling #386
Conversation
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
@ahl because nexus refers to the generated type all the time, there is no more conversion
david-crespo
force-pushed
the
complicated-internal-params
branch
from
b5abcbe to
469d6c6
Compare
| } | ||
| } | ||
| } | ||
|
|
| @@ -1,15 +1,8 @@ | |||
| //! APIs exposed by the bootstrap agent | |||
| //! Response types for the bootstrap agent | |||
There was a problem hiding this comment.
Why pull apart inputs and outputs?
There was a problem hiding this comment.
Related: I asked this on chat, but I'll ask it more publicly -
Are "views" supposed to be 1:1 with response types, or with DB representations?
This PR implies the former ("response types") but #374 implies the latter ("Views are public lenses onto the DB models.").
I'm especially curious about responses that do not represent data stored in a DB - does that still make sense to store in "view.rs"? If so, would it be more accurate to call these "requests.rs" and "responses.rs"? And in that case, I think Adam's question still applies - why split 'em?
There was a problem hiding this comment.
You're right to point out the disparity — I'll update #374 to say they're response bodies, most of which are lenses onto DB models.
I'm really just following a convention set by the big fullstack frameworks in various languages — Rails, Django, Phoenix, Laravel — which all call response bodies "views", though usually those are HTML responses. In Rails they call the JSON serializers "serializers". I guess they call them serializers in Django too. They distinguish views and serializers from controllers (route handlers) and models.
They're not as consistent about what they call request bodies, probably because in the dynamic languages the request bodies don't have to be defined anywhere outside of the request handler.
requests.rs and responses.rs would be fine names, though they may obscure that we're talking about the bodies in both cases.
There was a problem hiding this comment.
I'm going to merge as-is because we can rearrange easily later. The hard part, as you'll see, is moving things out of common. Moving things around within sled-agent or within nexus is easy.
DiskEnsureBodyis a request body expected by a sled agent endpoint, so it should live in sled agent. When Nexus wants to talk about these types, it should refer to the generated client.Doing this as its own PR because it would be incredibly confusing in combination with the other one.
To me this simplifies things a lot and is a good example of the kind of complication we currently have that makes things hard to reason about.