-
Notifications
You must be signed in to change notification settings - Fork 55
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
Support parsing default responses #472
base: master
Are you sure you want to change the base?
Conversation
@Mobius5150 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
@microsoft-github-policy-service agree |
@Mobius5150 Thank you very much for your contribution! We will review this as soon as possible. This package is in maintenance mode, I am wondering whether it is your direct dependency or indirect transitive dependency, and whether you could consider moving to our newer version of packages that are under active development? |
Hi @jeremymeng , thanks for the response. this dependency comes via I haven't tried the replacement to Also FYI: tests were passing locally but it seems some config on the linter changed and it's running amok. Happy to rebase if that gets resolved in |
Could you please log an issue at https://github.com/Azure/autorest.typescript repo?
Right. @autorest/typescript v6 is the recommended one. Yes, its version is still in RC, but it has been stable for quite a while, and we use it to generate our @Azure SDK packages today. We should be releasing a GA version soon. We will be more than happy to help you if there's any issue blocking.
Look that the package version needs to be bumped to "2.6.3" in both package.json and lib/util/constants.ts The change itself looks fine to me, according to the openapi v3 spec. Adding @joheredi to have another look from codegen's perspective. |
@Mobius5150 thanks for sending out this change proposal. If I'm understanding correctly, the change stops the library from throwing an error when the service returns a status code that is not defined in the swagger, correct? I'm interested to see some code snippets of the scenario you are trying to address with this change, to understand better the intent. Also, as @jeremymeng mentioned we recommend using @autorest/typescript, the new version of the generator is stable and we have been releasing libraries using it for some time now. For some background on the current behavior: The OpenApi spec is a little bit ambiguous on what I think interpreting default as an error makes sense as thinking of unexpected successful responses feels a little odd, I can't think of examples where an API doesn't know what it will return if everything succeeds. The generated libraries should still deserialize the error, the difference is that it needs to be accessed within a catch block. |
This PR adds support for a
default
response per the Open API v3 specification.