Closed willscott closed 1 year ago
IMO we should try and keep Caboose as limited to Saturn concerns as we can afford to. This means trying to keep it out of the IPLD game as much as we can.
My understanding of what we need out of this interface boundary is:
So we can probably go with your initial proposal Fetch(urlPath string, cb func(io.ReadCloser) error) error
if there are sentinel errors returned from the callback to help Caboose figure out what to do/what went wrong, An alternative that's a bit more explicit might be:
Fetch(urlPath string) (data io.ReadCloser, saturnReportBack func(some enum or struct), err error)
where the report-back could be a channel, callback, or whatever and the thing that goes into the callback is whatever information Saturn wants/needs.
My thinking is that instead of the caller trying to communicate with Caboose about whether to do a retry via error sentinels they can just call Fetch
again with the same or different URL if they need more data and the error wasn't obviously something Caboose can handle (e.g. a 500)
Thoughts?
@aschmahmann both can make sense - the semantics of
Fetch(urlPath string, cb func(io.ReadCloser) error) error
make more sense in my head.
We expect there to be some level of failure cases where cb
would end up triggering multiple times. In the reportBack
alternative, the caller has to re-call Fetch
on a failure. That would mean caboose would have to implicitly keep state between the subsequent calls in a way that feels like it would get messy.
(e.g. we'd want Fetch
to hold some sort of caboose-internal state struct so that it can know the retry is a second attempt and to try the fallback L1 for that request.) Having that end up in the interface feels ugly to me.
I can prototype Fetch(urlPath string, cb func(io.ReadCloser) error) error
and we can see if it's painful to deal with?
Yeah, I'd hope the implementation internally isn't so dependent on this that we can change it later if need be. While you're prototyping try and keep in mind what you want the caller contract to be when the errors are streaming (i.e. interrupted CARs) rather than 4xx/5xx errors.
I went with the interface
type ErrPartialResponse struct {
error
StillNeed []string
}
As the way to handle partial responses - the cb
would return this error with the sub-paths of data remaining as the way to efficiently handle resumptions.
One thing this likely means is that i'm expect to update as i get through tests and support for efficient handling of those partial continuations is that the callback will evolve to
cb func(path string, reader io.ReadCloser) error
so that the subsequent invocation knows the subset the returned reader is fulfilling.
This should be enough for initial integration in starting to craft what the car validation / fullifilling callback looks like.
the first top-level interface of caboose is as a blockstore.
we should work with @aschmahmann to understand what requests for car files rather than blocks might look like as they pass through this library.
My first inclination is something like
where
cb
would validate if a response matched expected response (and would be called 0+ times, and at most 1 time where it returned anil
error)