Issues with the GOLANG Onshape APIs

[toebes]

I’ve been attempting to use the Onshape GO APIs and am slowly coming to the conclusion that it would be nicer if they utilized a different strategy for GO. Basically there are two issues to consider here that I’ve described in detail below:

1. Context/Client access to the APIs
2. Stub appropriateness for use

Context/Client access to the APIs

It isn’t obvious why there is both a client and a context object for accessing the APIs. It basically means that every routine needs to have two pieces of information passed to them if they are to access the APIs. Now it certainly is possible that someone could just create them as global variables, but that of course is not good programming practice. For example, the typical application will do something like:

    config := onshape.NewConfiguration()
    config.Debug = onshapeDebug

    client := onshape.NewAPIClient(config)
    if apiSecretKey == "" {
        apiSecretKey = os.Getenv("ONSHAPE_API_SECRET_KEY")
    }
    if apiAccessKey == "" {
        apiAccessKey = os.Getenv("ONSHAPE_API_ACCESS_KEY")
    }
    if onshapeDebug {
        fmt.Printf("Keys: Secret=%v Access=%v\n", apiSecretKey, apiAccessKey)
    }

    ctx := context.WithValue(context.Background(), onshape.ContextAPIKeys, onshape.APIKeys{SecretKey: apiSecretKey, AccessKey: apiAccessKey})

Once that has been done, every call needs to be referenced from the client and the first parameter passed is the context. If you look into the client and context structures, you find that the context is the typical GO context (https://golang.org/pkg/context/) which allows for storing all sorts of useful data. On the other hand, the client object is not much more than a pointer to all of the function groups and the config object. Examining the information stored in the config object you find some HTTP header/agent information and the Debug flag, all of things that could be stored in the context object using the WithValue() method.

Furthermore, having the client object provides for some unwieldy access to the APIs. Not only does a developer have to look up the name of the API, they also have to look up the category to call it. Currently there are 193 API entries in 26 different classes (three of the classes have only one API entry and three more only have two) and there are no conflicts on names for any of the APIs. So instead of a developer calling:

docInfo, rawResp, err := client.DocumentsApi.CreateDocument(ctx).BTDocumentParams(*docParams).Execute()

it would make much more sense for them to call:

docInfo, err := CreateDocument(ctx).BTDocumentParams(*docParams).Execute()`

Stub appropriateness for use

The current stubs require a lot of overhead on the part of the caller. In addition to juggling the parameters into place, the caller has to handle quite a few error cases. Here’s an example of a typical call to just get the metadata for a documemt:

    // Get the Metadata for the document.  This returns the list of lower level tabs in the document
    // fmt.Printf("Calling: /api/metadata/d/%v/w/%v/e\n", *did, *wvid)

    // Declare the variables that we will be filling in during the loop.
    var MetadataNodes onshape.BTMetadataInfo
    var rawResp *http.Response
    var err error
    // Arbitrarily we aren’t going to try more than 50 times. In the worst case we will be delaying 2.5 seconds between calls 
    for delay := 0; delay < 50; delay++ {
        MetadataNodes, rawResp, err = client.MetadataApi.GetWMVEsMetadata(ctx, *did, "w", *wvid).Depth("5").Execute()
        // If we are rate limited, implement a backoff strategy
        // We only find out that it is a backoff if the error string has a 429 followed by a single space.
        if err == nil || err.Error() != "429 " {
            // Not a rate limit so we can stop retrying
            break
        }
        // Rate limited, implement a progressive backoff adding 50ms each time through the loop.
        fmt.Printf(".......Rate Limited.. Sleeping\n")
        time.Sleep(time.Duration(delay*50) * time.Millisecond)
    }
    // Check to see if we have any actual errors returned
    if err != nil {
        // Propagate the error to the caller
        return err
    // Check to see if we had a HTTP response code of 300 or higher which indicates a failure
    // https://en.wikipedia.org/wiki/List_of_HTTP_status_codes
    // but which wasn’t returned as an error code from the lower level API
    } else if rawResp != nil && rawResp.StatusCode >= 300 {
        // The API actually failed, so format an error code to propagate up
        err = fmt.Errorf("err: Response status: %v", rawResp)
        return err
    }

What would make a lot more sense for general usage would be a call

   MetadataNodes, err = GetWMVEsMetadata(ctx, *did, "w", *wvid).Depth("5").Execute()
    if err != nil {
        return err
    }

In this case, the stub should handle the 429 and the 300 errors returning only an error and not the _nethttp.Response code. It would also ensure uniform processing of any back-off strategy to ensure optimal use of the Onshape APIs. I’m happy to create a stub that encapsulates all of this, but it feels odd to build a shim layer on top of another shim layer (which is actually layered at least one more level deep).