Skip to content

Add RetrieveFrom, to read a file from an offset - #81

Open
christhomas wants to merge 1 commit into
secsy:masterfrom
antimatter-studios:cth/retrieve-from-upstream
Open

Add RetrieveFrom, to read a file from an offset#81
christhomas wants to merge 1 commit into
secsy:masterfrom
antimatter-studios:cth/retrieve-from-upstream

Conversation

@christhomas

Copy link
Copy Markdown

Implements the request in #47.

The plumbing was already there: Retrieve uses REST to resume a transfer that failed part way, so transferFromOffset already does this work. This exposes it. Retrieve becomes RetrieveFrom(path, dest, 0), with a test asserting its behaviour is unchanged.

On the API shape

You suggested:

What if Retrieve checks if dest is an io.Seeker and if so sets bytesSoFar to the current offset? Or we could add a wrapper type to make this behavior opt-in

I went with the explicit method @gauravtiwari-tw asked for. A Retrieve that starts somewhere other than the beginning depending on the concrete type of its argument is harder to reason about than a second method — the caller has to know that *os.File behaves differently from a bytes.Buffer — and a wrapper type is a third concept to learn for something a parameter says plainly.

Happy to swap it for either of yours if you'd rather; the underlying change is one line either way.

Four cases decided here rather than left to the server

offset behaviour why
== size copies nothing, no error where a caller resuming an already-finished transfer lands; also avoids asking a server for zero bytes, which invites a server-specific answer to a question with an obvious one
> size error server replies vary and some send nothing, which is indistinguishable from success
< 0 error never reaches the wire as REST -1
> 0 without REST STREAM error Retrieve merely loses the ability to retry; this cannot start at all, so it says so rather than quietly sending the whole file from the beginning

One note on running the tests: this branch is off master, so TestMain still requires ./build_test_server.sh. I verified via the container harness in #73.

Requested in secsy#47.

The plumbing was already there: Retrieve uses REST to resume a transfer
that failed part way, so transferFromOffset already does the work. This
exposes it. Retrieve is now RetrieveFrom starting at zero, which is
covered by a test asserting its behaviour is unchanged.

The maintainer suggested inferring the offset from dest when it is an
io.Seeker, or an opt-in wrapper type. This is the explicit method the
issue asked for, because a Retrieve that silently starts somewhere other
than the beginning depending on the concrete type of its argument is
harder to reason about than a second method — and the wrapper type is a
third concept to learn for something a parameter says plainly.

Four cases are decided rather than left to the server:

An offset equal to the file's size copies nothing and is not an error.
That is where a caller resuming an already-complete transfer lands, and
failing them would be unhelpful. It also avoids asking a server for zero
bytes, which invites a server-specific answer to a question with an
obvious one.

An offset past the end is an error. The server's own reply varies and
some will happily send nothing, which is indistinguishable from success.

A negative offset is an error, and never reaches the wire as "REST -1".

An offset above zero against a server without REST STREAM is an error.
Retrieve merely loses the ability to retry in that case; RetrieveFrom
cannot start at all, so it says so rather than quietly sending the whole
file from the beginning.
@christhomas christhomas mentioned this pull request Aug 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant