sdk-codegen
sdk-codegen copied to clipboard
IDs are incompatibly typed: Sometimes strings, sometimes ints
For the Python SDK, it appears that there is some incompatible typing going on. This is for both SDK 3.1 and SDK 4.0.
Take folder_id
, for example. In the model, the id is an Optional[str]
; but in e.g. copy_look
, it requires int. These issues cause mypy to fail when using the sdk. It's hard to know whether or not the str
and int
types are always compatible; that is, if the id is present, whether it can be safely casted to a string. To start with, it might be helpful to document this in the SDK.
thanks for the report. I'm following up with the copy_look endpoint author to understand why they're asking for an int instead of a string.
I actually came here to raise the same issue, but in relation to the Go SDK.
As @fbertsch mentioned, the API model for Folder has id
(and parent_id
) as having the type string
, however the JSON response returns an integer for these values. When creating a folder using the Go SDK, the type discrepancy ends up causing a json.unmarshalTypeError
and the resulting object that's returned has empty strings in place of the two fields above in lieu of their actual values.
As an aside, the above error is currently going unchecked by the Go SDK, but I can see this has been flagged already in #520.
It'd be good to get clarity on what the types should be and update the SDK or API docs accordingly. I'm happy to help contribute a PR when the time comes.
Model schema per API docs:
{
"name": "string",
"parent_id": "string", 👈
"id": "string", 👈
"content_metadata_id": 0,
"created_at": "2021-04-28T06:35:32.302Z",
"creator_id": 0,
"child_count": 0,
"external_id": "string",
"is_embed": true,
...
}
JSON response from GET /api/3.1/folders/1
:
{
"name": "Shared",
"parent_id": null,
"id": 1, 👈
"content_metadata_id": 1, 👈
"created_at": "2020-02-03T19:44:00.514+00:00",
"creator_id": null,
"child_count": 4,
"external_id": null,
"is_embed": false,
...
}
Go model:
type Folder struct {
Name string `json:"name"`
ParentId *string 👈 `json:"parent_id,omitempty"`
Id *string 👈 `json:"id,omitempty"`
ContentMetadataId *int64 `json:"content_metadata_id,omitempty"`
CreatedAt *time.Time `json:"created_at,omitempty"`
CreatorId *int64 `json:"creator_id,omitempty"`
ChildCount *int64 `json:"child_count,omitempty"`
ExternalId *string `json:"external_id,omitempty"`
IsEmbed *bool `json:"is_embed,omitempty"`
...
}
@christippett the issue of id and parent_id being advertised as strings and coming back as integers in API version 3.1 and many similar discrepancies across other models is the main motivation we released API version 4.0 which behaves correctly (says string and sends string). I'd highly recommend using 4.0
we may change the spec for 3.1 in cases like this (and advertise an integer instead of a string, we'll never change the actual returned value as that would be a breaking change) but it's not a high priority so, again, 4.0 is your friend :-)
wrt to #520, that actually seems to be an issue with the RTL (run time library) and a patch to fix it would be most welcome!
@joeldodge79 should we be switching to 4.0 in production services?
@fbertsch that depends - we hope to keep 4.0 relatively stable until we make it GA. But, as you can see some things will be changing as we find bugs/inconsistencies like this one (the folder_id
argument to {copy,move}_{look,dashboard}
will be changed soon to be a string).
If you need very high stability and can live with some type mis-matches (e.g. spec says it's a string but the payload contains an int) then 3.1 is your choice. That being said, many new methods (including these in particular) are only available in 4.0
Gosh, I feel silly now. I completely neglected to consider the 4.0 version of the API, thank you for pointing it out to me @joeldodge79. I'll see myself out...
To echo @fbertsch's question back in April - should we be using the beta-quality v4 SDK for production services? We definitely need to access folder IDs through the SDK, but at the same time we need to base our product on a stable library.