Open mikefarah opened 8 years ago
I was wondering when webhooks were going to pop up as an issue, so congrats on that :)
First off, before we go into solutions mode, I'm curious as to why you think this is iffy in your current approach. I'd also like to know how bi-directional pacts is working for you at REA.
My personal thoughts behind it is that I wouldn't want to create a single pact contract for 2 separate interactions. The reason for this is that we would lose testing granularity and essentially mean that one is dependent of the other, which it isn't because anyone should be able to post to /alligation/response with a message.
What I could be convinced to do is a helper function that would create a new interaction, that way you don't have to duplicate code to define it, but that causes issues around the naming since even if given 'an alligator exist' and receiving 'a request for an alligator' isn't the same as what the definition of the webhook is.
Essentially, this comes down to your server being both a provider and a consumer, something that I haven't had the pleasure of dealing with a lot with Pact at the time being (we mostly use pact between front-end and API, so a 1 to many relationship for consumer/provider, but the APIs themselves don't use pact between them). Maybe all we need to do is create an easier workflow between consumer and provider. To do that, we would have to know what is your current workflow. Any information on what your process is, what problems are you solving (abstract, no details) or even example could be greatly helpful for the future of the foundation.
Thanks for your comment.
Sure, so the reason is seems iffy is because in my mind we're using webhooks as a method of getting the result of the original request. That is, if you want to get details on the alligator, make a GET request to alligator with a callback url, and we'll call you back with the details. It's the single contract on how to get alligator information from the service provider (even though the provider makes a request back to the consumer). The callback body specification therefore should be consumer driven, not provider driven - just like it would be had it been a synchronous request.
Currently, we have two contracts and this separation means that it's not obvious that the requests are actually part of the same transaction (from a logical point of view). Half the contract is on the consumer, and the other half on the provider.
Btw I'm not currently at REA (although I did work there with some pact goodness). We've just started working on the bi-directional PACTs and I'm working on the provider side.
Thanks for getting back to me quickly. Okay, so now I'm curious as to why you're using a callback when it sounds to me that you're just getting the response synchronously. The whole point of a webhook is a set and forget until it hits back. I understand that in your mind they're related (and they are) but the abstract interface is asynchronous and shouldn't know about the data coming back from your webhook since that's a separate interactions. The first one is just a 'register webhook', the second is a 'getting data from webhook' interaction. It sounds like you're trying to get data in a synchronous way but using an asynchronous API to get it, which to me isn't really what Pact is trying to do.
I think the issue here is more around your usage/implementation of the API interaction than it is for Pact to handle. Since you do have a working solution, I'm going to tag this as low priority for now while I ponder about your problem a bit further. To me however, if someone on my team were asking me this, I'd question why they aren't using a normal HTTP call. If they wanted the data now, plus listen for any new update on that data, I would make 2 different calls, one to get the current state using a GET, and another to register the webhook. Potentially, this API call could be one of the same. The interaction between both however, are very different.
Again, I don't think this issue is one of specification, but implementation. I think this could potentially be done with a better library DSL with a helper function for callbacks that creates that interaction for you, something along the lines of:
animal_service.given("an alligator exists").
upon_receiving("a request for an alligator").
with_callback(method: :get, path: '/alligator', query: 'callback=/alligator/response').
will_callback_with(status: 200, callback: { method: :post, path: '/alligator/response', body: {name: 'Betty'}})
I'll have to consult the others to see what are potential solutions, but this wouldn't be done as a priority since we have a slew of other things to address first before tackling this. I hope you understand.
Fair enough, just wanted to put it out there to see what other people have done, whether it's recurring issue etc.
The reason it's not synchronous is that the request takes time (~ an hour) to complete.
Register webhook sounds like a bit more like a publisher/subscribe model which is not quite what this is.
Anyhow, perhaps this is a bit more unique than I originally thought. Thanks for the feedback!
It's not that it's that unique. We've talked about it internally a while back as to what we'll support; for now we've only concentrating on http requests while we're redoing the library and try to unify the codebase to help us move faster and improve the developer experience. Then we can concentrate on new features like webhooks or websockets.
I hope you understand.
On Wed, Apr 27, 2016, 9:22 PM Mike Farah notifications@github.com wrote:
Fair enough, just wanted to put it out there to see what other people have done, whether it's recurring issue etc.
The reason it's not synchronous is that the request takes time (~ an hour) to complete.
Register webhook sounds like a bit more like a publisher/subscribe model which is not quite what this is.
Anyhow, perhaps this is a bit more unique than I originally thought. Thanks for the feedback!
— You are receiving this because you commented.
Reply to this email directly or view it on GitHub https://github.com/pact-foundation/pact-specification/issues/26#issuecomment-215053870
Hmm actually the workaround of writing a seperate pact file won't work, as the provider does not have a fixed callback path to callback on. The callback path is specified on the original request.
Now the provider could, write out seperate webhook pact files for every consumer, but that would mean we would need to hard code knowledge of each consumer in the provider, and introduce a bi-directional dependency. Not cool.
Not sure I'm understanding your dilemma, could you please try to explain it better? Maybe a quick diagram would help.
On Fri, Apr 29, 2016, 2:14 PM Mike Farah notifications@github.com wrote:
Hmm actually the workaround of writing a seperate pact file won't work, as the provider does not have a fixed callback path to callback on. The callback path is specified on the original request.
Now the provider could, write out seperate webhook pact files for every consumer, but that would mean we would need to hard code knowledge of each consumer in the provider, and introduce a bi-directional dependency. Not cool.
— You are receiving this because you commented.
Reply to this email directly or view it on GitHub https://github.com/pact-foundation/pact-specification/issues/26#issuecomment-215622804
Well if you look at the original example, the callback url is passed in the first request. This callback url will vary from consumer to consumer, and could very well be a generated RESTful url, e.g. call me back on /alligator/12. The URL is controlled by the consumer, the provider does not care or know the details.
To write the webhook PACT however, the PACT file must specify the URL - which means now the provider needs to have intimate knowledge of its consumers.
It is quite suprising that that PACT doesnt support this. They are now part of the OpenAPI spec. See https://swagger.io/docs/specification/callbacks/
We don't support OAS either. Pact is agnostic to specifications such as OAS/SOAP/AsyncAPI etc.
Yea, I am aware of that. I was meaning that this type of interactions between services is now standardised, but Pact cant be used to test them. IIUC, Async non-HTTP messages have been added to Pact in that time, so it is surprising that Async HTTP messages havent yet made the cut to get standardised.
This is the only issue in the repo tagged with "low-priority" - perhaps that label could be re-evaluated in this context.
This repo doesnt give clues on how we could help. I am guessing a new version is at least on the drawing boards somewhere.
I'd be interested in @uglyog's perspective on how the new V4 specification abstractions may be able to handle this use case.
V4 Pacts can support different types of interactions in the Pact file, but there is not any way to chain two different requests together. The original issue was describing a request from A -> B, and then validating the later request from B -> A as part of the same integration. Pact still sees these as two separate interactions.
We have also been looking into the type of interaction where A sends a request to B to start a background process, and then later waits for a message from B on a queue. Again, this is chaining two requests together to make one interaction.
Given the broader abstraction needed here, i've updated the title to reflect that need.
moved from https://github.com/realestate-com-au/pact/issues/103 We are using PACT goodness for testing our micro-service interactions, and one of our services uses webhooks to handle asynchronous, long running tasks.
Currently, the only way to PACTify this (as far as we know) is to have bi-directional pacts, one for the consumer to the provider for the original request, and one from the provider back to the consumer to the webhook callback.
This feels a littly iffy to me, I'd be happier with a single PACT file that goes along the lines of:
`
An immediate issue I can see (going from the example in the readme) is that the test is for the AnimalServiceClient wheras the callback would hit the controller of the consumer service.
Thoughts/Tips? Anyone done anything similar