Go all-in on HTTP API #53
Conversation
|
looks good to me but it's only part of the puzzle right? in a single-node setup this code won't create a bus/worker client to pass into the autopilot |
| } | ||
| return s, nil | ||
| return s, length, nil |
There was a problem hiding this comment.
Any reason you are returning the length here? Seems like we're never using it.
There was a problem hiding this comment.
it's used in objectsKeyHandlerPUT.
It's not strictly necessary -- we could wrap the io.Reader in a counter to determine the total length of the object -- but this is easier.
| @@ -190,3 +190,11 @@ type MigrationContract struct { | |||
| HostIP string `json:"hostIP"` | |||
| ID types.FileContractID `json:"id"` | |||
| } | |||
|
|
|||
| // UploadParams contains the metadata needed by a worker to upload an object. | |||
| type UploadParams struct { | |||
There was a problem hiding this comment.
Slightly different than what I had in mind. I figured the worker would be using contract sets? I also figured the redundancy would be optionally provided (defaults to 10-30) by the user on the upload endpoint.
Seeing as you fetch it from the bus I assume you have in mind to add it to the bus config? So if I want to upload an object with a different redundancy I'd have to manually use the slabs endpoint or change my bus config?
There was a problem hiding this comment.
The redundancy is also part of the autopilot config, so the migrate endpoints at least should expose some way of specifying the required redundancy.
There was a problem hiding this comment.
Sure, we can allow overriding the redundancy settings. I'll wait to add that until it's needed, though.
The bus should store the default redundancy. If the autopilot is currently storing the redundancy settings, we should change it to fetch the settings from the bus.
|
|
||
| // UploadParams contains the metadata needed by a worker to upload an object. | ||
| type UploadParams struct { | ||
| CurrentHeight uint64 |
There was a problem hiding this comment.
Isn't it a bit awkward to have consensus logic in the upload endpoint? What happens if the height changes during the upload?
There was a problem hiding this comment.
The current height is required in order to calculate how much time is left in the contract, and thus how many blocks of storage we're paying for. If the height changes mid-upload, we might slightly overpay or underpay; IIRC the renter has some leeway to account for this.
I have always been annoyed that we have to know the height in order to upload. Would be cool if we could remove it in RHPv4 somehow.
workerandbusare proper microservices now: the only way to access their functionality is with aClient. This spares us from reimplementing every method twice. This does introduce some overhead for local setups, but we'll wait until we've measured it before making any decisions around refactoring.