nebulouslabs / go-skynet Goto Github PK

We should know the StatusCodes we should receive and handle them accordingly. Something like a switch statement that handles different codes explicitly and gives a developer error if we have missed one?

Otherwise if that is over the top I think we should explicitly check for success and return an error if not so we aren't hiding any weird status codes that might create UX bugs or inconsistencies for users.

See #10 (review)

I'm thinking we treat >= 200 and < 300 as success and anything else as unexpected and therefore an error.

In the python sdk, there used to be an option for http/1.1 chunked uploading. This way of posting a single block of data provides for streaming uploads where the size of the data is not known in advance. (I was actually using this feature and trying to port it forward in the python sdk)

I didn't find an interface for this in the go sdk to copy to the new python sdk, but the servers seem to still support it fine. Is there a plan for this feature?

I simplely run the sample code like this .

package main

import (
	"fmt"

	skynet "github.com/NebulousLabs/go-skynet"
)

func main() {
	// upload
	skylink, err := skynet.UploadFile("./src.jpg", skynet.DefaultUploadOptions)
	if err != nil {
		fmt.Printf("Unable to upload: %v", err.Error())
		return
	}
	fmt.Printf("Upload successful, skylink: %v\n", skylink)

	// download
	err = skynet.DownloadFile("./dst.jpg", skylink, skynet.DefaultDownloadOptions)
	if err != nil {
		fmt.Printf("Something went wrong, please try again.\nError: %v", err.Error())
		return
	}
	fmt.Println("Download successful")
}

But after , build and run , one error happened. Any one can tell me why .

Error Content :

Unable to upload: [could not execute request; error code received; 400 response from POST: failed parsing Content-Type header: mime: no media type; error response]

Unable to upload: invalid character '<' looking for beginning of value

From #10 (comment):

Looking at this MR more I'm wondering if it makes more sense to rename this to APIRoute and add it to the ConnectionOptions.

That would allow us to generalize the code even more. Unless there is a plan to move the parameters like name, id, skykeytype into their corresponding options struct it seems unnecessary to be defining an Options struct for each endpoint.

Then this line becomes even more clear

url := makeURL(opts.PortalURL, opts.APIRoute)

To be able to define the defaults easily we could have just two structs.

ConnectionOptions struct {}
APIRouteOptions struct {}

I'm thinking that there are a number of common API parameters that could be included in the APIRoutes eventually, like async root force etc.

Then the defaults can be defined as

DefaultGetSkyKeyOptions = APIRouteOptions {
  ConnectionOptions: defaultConnectionOptions,
  APIRoute: "/skynet/skykey",
}

My thoughts:

I think we could have the APIRoute as a field in ConnectionOptions which we rename to just Options and the options for each endpoint can compose on top of it. I want to keep a separate option struct per endpoint though for better type-safety and future-compatibility. It would also be weird if AddSkykey took an Options while Upload took UploadOptions

I'm thinking that there are a number of common API parameters that could be included in the APIRoutes eventually, like async root force etc.

Most of these don't apply to many of the endpoints. This is what we do in Python and Javascript atm unfortunately. But I would like to restrict the users a bit e.g. they shouldn't be able to set force if the endpoint doesn't take it.

I was copying some of the upload interface into the python sdk, and I noticed that when only one file is being uploaded, the filename url parameter is passed but not given any content. It's around this area of code.

If you look at that if block, you'll see only fieldname is set, unlike the else condition, but right below there is:

filename is used but was never set. I'm not familiar with go, but I'm guessing this variable likely contains an empty string here, possibly whatever was last in local memory where it's allocated.

When copying to python, I figured it was safer and clearer to give this parameter the actual filename, but I don't know for sure if that is the correct behavior. Does the server ignore the filename parameter when there is one named multipart file? Does it fill it in automatically when it's empty? Or is this a bug?

I've seen in my metadata queries that many records do have the filename listed twice: once on the whole record, and again within the content alongside the size. So I figured the query parameter might set the outer name, but I don't really know. Is this documented concisely somewhere?

EDIT: I've significantly reworded the above.

Should add a README with an overview of the supported actions

Currently UploadOptions is not useful because in your own code you can't create an instance with different options than the defaults, because all of the fields are unexported.

For example, to change the portal URL to my own, I would want to update portalUrl, but since it is lowercase and thus unexported, this will fail to compile:

var customUploadOptions = skynet.UploadOptions{
	portalUrl:                    "https://www.siacdn.com",
	//portalUploadPath:             "/skynet/skyfile",
	//portalFileFieldname:          "file",
	//portalDirectoryFileFieldname: "files[]",
	//customFilename:               "",
}