-
Notifications
You must be signed in to change notification settings - Fork 123
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
POST /v1/subscriptions missing a required parameter? #127
Comments
@stepcut there are legacy integration paths that exist on that API that still work but are undocumented. So in a way both are kind of correct. The openapi spec is "technically correct" because our API does support paths where As a developer, you should assume that I'll see internally if we can just change the openapi spec to reflect the newer default behaviour as it's been over 5 years since we introduced I hope this helped clarify the overall state but let me know if not. |
Thanks! This is helpful. In case you are curious -- here is more information about what I am doing. First off, I'd like to say that I am really impressed by your legacy integration. I am using a Haskell Stripe binding that uses a 2014 version of the API and it still mostly works. But obviously we should upgrade to a more recent version of the API. The old version of the Haskell bindings predate the OpenAPI spec and I am working to modernize the bindings. My initial hope was that I would be able to generate bindings directly from the spec -- but the deeper I get into the project, the less that seems reasonable. It sounds like the most up to date information is still only available as human readable documentation? As this binding generator progresses, it contains more and more knowledge that I have encoded by hand, and the spec is often used just to verify that my bindings are still current. In this case, my bindings will need to manually encode the fact that both I am willing to document the various obstacles I have run into trying to generate a binding directly from the specification, but I suspect that the changes which need to be made to the spec to accommodate that are too significant to actually make it on the project plan. |
@stepcut Oh that sounds really interesting! The openapi spec is mostly correct and all our client libraries/SDKs today are generated from that exact openapi spec too. We do have some manual "overrides" to try and remove/deprecate old stuff we want to get rid of but it's quite rare. Have you tried looking at the spec we have specifically for our client libraries/SDKs which is spec3.sdk.yaml (or the JSON version)? Overall, we do recommend avoiding encoding things like this in your library for now. If you look at our own type definitions in stripe-dotnet or stripe-node for example we don't really enforce this in our types signatures and it works fine. |
@stepcut I'm curious, are you building a community library and your vision is to supplant e.g. https://hackage.haskell.org/package/stripe-haskell? Or are you building it mostly for use in your own project? If you are building a community library I would strongly second Remi's suggestion that you trust the OpenAPI spec and avoid encoding knowledge by hand. As Remi said, the OpenAPI types might appear to be more permissive in some places than they should be, but in each of those cases there is some scenario or edge case that prevents us from using a stricter type, so if the library is for general use you wouldn't want to preclude those. As Remi hinted by mentioning our "overrides" that we have for the generators of our official SDKs, if you do want to make customizations to the types described by OpenAPI -- my advice is to express those as modifications to apply either to OpenAPI before your generation code runs, or to apply to a representation of your generated output immediately before you render it, vs. what you seem to be describing -- manually making changes to the generated output and then checking it against the spec to find inconsistencies. In any case, please don't hesitate to reach out to us here to ask us for advice/feedback/any sort of guidance! We are excited you are working on a Stripe library and helping community libraries be successful is a goal of ours. I personally am quite fond of Haskell, too! |
I am a co-author of that I have been using Both For example, the
This says that I have looked at the official client libraries and they do seem to give the users a lot of rope to hang themselves. If I was to model my library after the existing libraries then a user wanting to use
While that code might compile it is riddled with errors:
Each of these errors would result in a runtime error and a tedious debugging cycle. But eventually they would get it work. So by some definitions it 'works fine'. However, in the old handwritten version of the Haskell Stripe library, we can detect all these mistakes at compile time by making use of the type system.
In this more Haskelly version:
The Haskelly version adds a ton of type safety but is just as concise. Unfortunately, even in
So even reading the documentation does not make it clear what format is expected for In the Haskell Stripe library, we can encode the fact that the Our code generator can check that the definition in the specification hasn't changed. If the spec does change, then we have to verify if the changes are compatible or not and potentially update the code generator. The spec also has this curious
I think what that means is that you can pass in an empty string instead of an empty list and it does something different? One thing I would love to see in the spec is that ids like
And then referenced everywhere they are used like this:
This would also help clarify endpoints like this one,
what is This seems a lot more useful,
With these changes, you could still generate the current bindings for Python, Node, etc, which make heavy use of untyped strings everywhere. But it would also make it far easier to create bindings that make better use of the type system in languages that have reasonable type systems. tl;dr -- should I ignore the advice in |
This is an excellent point and thank you very much for the suggestion. We haven't put this information in the OpenAPI spec, probably because none of our target languages have anything ergonomically similar to Haskell's "newtype", but come to think of it we could use template literal types in Typescript. Some of our official libraries do use types to prevent some of the problems you describe. We have Typescript in stripe-node now, which prevents field misnamings, mistypings, captures requiredness, etc. Stripe-Java has typed params via builder methods now (this may not have been the case when stripe-haskell originally cropped up), which can't help with requiredness but does prevent most of the other problems. Our dynamically-typed languages still have a lot of problems that you describe but we're working on introducing gradual types, which will help. In any case, with the exception of id types more specific than string, our Typescript bindings provide all of those safeguards, and we generate that from The reason we have that disclaimer for
A lot of this you may be able to do without. You might be able to name and group methods solely by the HTTP paths, for instance, but we need that information because we hold ourselves pretty heavily to backwards-compatibility.
We call that "emptystringable" or "emptyable". anyof("", X) indicates that you can send "" to the API in order to "empty" the value there.
If you find a use for it you can, but just keep in mind there's no stability guarantee, so you're signing up for work if/when we iterate on the metadata there. One way to kind of protect yourself from changes there -- and what we do in our own generators -- is we immediately parse OpenAPI into our own internal representation that represents exactly the information we need to generate the library in a much more convenient form. |
Also, would you be willing to open up a new issue (with the "feature request" label) asking for more granular "id" types? You can retitle this one, too. |
Oh, @remi-stripe has reminded me that
is NOT actually a good idea because we actually do change the prefixes sometimes and in some cases allow users to specify their own IDs. What you are suggesting still works though -- maybe starting with |
According to this documentation,
https://stripe.com/docs/api/subscriptions/create
When doing a
POST
to/v1/subscriptions
thecustomer
anditems
parameters are both required.But in
spec3.json
it merely specifies,Which is correct -- the spec or the documentation?
The text was updated successfully, but these errors were encountered: