-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Incorrect return type definition for deleteKey
#11
Comments
This scenario is an unfortunate consequence of the API definition containing multiple success responses for the deleteKey operatoin... a 200 which returns a response object, and a 204 with no response object. |
Hi @padamstx while I understand the technical reason for this, I think that this aspect still should be addressed, because imho we should make sure that the type definition of the API matches the real data structures. Otherwise how would I code against the API? I would have to code with the premise in mind that there exists a mismatch between the API and the data. In that case it would be better to declare the response as From your explanation the root cause for this issue is a shortcoming of the API generator. I have not looked into every detail, but based on the description a better type definition would be: deleteKey(params: IbmKeyProtectApiV2.DeleteKeyParams): Promise<IbmKeyProtectApiV2.Response<IbmKeyProtectApiV2.DeleteKey | string>>; this is still not perfect, because it does not define the circumstances under which you'd expect a string vs a data structure, but it would at least give a hint to expect a string. |
In this situation, the service team really can't update the API definition, because it currently reflects the behavior of the service (i.e. the operation can return a 200 status code along with an instance of the DeleteKey schema in the reponse body, or a 204 status code with no response body at all). |
I do not want to ride this to death, but why can't we fix the SDK generator? If there exist languages that do not support type unions, then the correct type for |
The primary reason is that our SDK generator is built on the openapi-generator open source code, and this behavior comes from that underlying code. I realize that's still not a "good" reason. Perhaps in the future we can look at modifying this behavior, but it would require a fairly large effort due to the need to address first in the underlying openapi-generator code.
I think it's even easier than that... i.e. if you receive a 200 status code (you can check this by looking at the "status" field of the response object returned to you), then you should be able to assume that the response object's "result" field contains a non-null instance of the return type (DeleteKey); OTOH if you receive a 204 status code, then you can assume there's no "result" value returned in the response. This is essentially how the operation is defined in the API definition, although it is not clearly implemented that way in the generated SDK code. |
The type definition for
deleteKey
is:with
IbmKeyProtectApiV2.DeleteKey
:but the actually returned data is:
Clearly
result
is not of the specified data type.The text was updated successfully, but these errors were encountered: