Skip to content
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

Reuse provider verification result in a single run #239

Open
ertrzyiks opened this issue Mar 11, 2021 · 7 comments
Open

Reuse provider verification result in a single run #239

ertrzyiks opened this issue Mar 11, 2021 · 7 comments

Comments

@ertrzyiks
Copy link
Contributor

ertrzyiks commented Mar 11, 2021

Let's say our contract file has 100 interactions.

When we create a feature branch adding 1 more interactions we have 2 contracts:
A. 100 interactions
B. 100 interactions from contract A + 1 new interaction

The provider verification picks both contract files (either specifically, either as WIP). Currently, all 201 interactions are checked even 100 of them are verified twice. Could we reuse a single interaction verification results to optimize it, so only 101 interactions are actually performed?

For the background, when we have multiple WIP branches our job execution time jumped from 9 minutes to 50 minutes while most of the work is doing the same thing over and over.

@mefellows
Copy link
Member

I think this has come up before, at the very least, I've raised this idea before (not sure... where, probably on pact-broker). Beth did some investigation a while back using data from our Pactflow customer base. We resolved that it wasn't a big saving.

That may have been before WIP/Pending though, which I can see would change this situation.

In any case, keen to see if others have this problem too.

(Also, probably should be moved to pact-broker if it's not a dupe)

@bethesque
Copy link
Member

I thought I'd already created a feature request for this somewhere, but I can't find it. I've created it here: https://pact.canny.io/feature-requests/p/aggregate-pacts-with-duplicate-interactions-into-a-single-uber-pact-to-reduce-ve

I did some numbers on this, and determined that for a small number of users, it would make a big difference, but for the majority, it wouldn't make much difference. You're obviously in the small number of users for whom it would make a big difference @ertrzyiks. 50 minutes is just ridiculous. Is this why you're keen to get that wip selectors thing? What's the simplest thing I can do to make that quicker?

@bethesque
Copy link
Member

@ertrzyiks are you across this? pactflow/roadmap#4 If your builds are taking that long, they might be a good candidate for bi-directional contracts.

@ertrzyiks
Copy link
Contributor Author

ertrzyiks commented Mar 12, 2021

Thanks for the fast reaction @mefellows @bethesque !

@bethesque I was not aware of bi-directional contract, I need to read about it, thanks for the hint.

Is this why you're keen to get that wip selectors thing?

It's part of the problem, yeah. We wanted to evaluate if enabling WIP verification is feasible in our system, so the initial plan was to introduce it just for one pair Provider-Consumer. Another thing is that some of our Provider-Consumer pairs are just being set up so the broker is filled with more data than we care about at that point. WIP selector picks contracts that we then never check with can-i-deploy and it is waste of resources.

What's the simplest thing I can do to make that quicker

I created this issue in the ruby gem because the system in question uses ruby. If you are unsure if this feature should make it to the pact spec for all the languages, maybe we can have an option to enable optimization like enable_experimental_uber_pact and skip interactions that were already checked? If you think it makes sense I can try to submit a PR, at least with some draft idea.

For now, we disabled WIP verification so the job is fast again. It reintroduced the issues we aimed to fix with WIP, so eventually it would be great to bring WIP verification back but the performance issue is a real pain to us.

@ertrzyiks
Copy link
Contributor Author

@bethesque any thoughts on how / if we want to push this idea forward? I briefly checked the bi-directional contracts, but I see it more suitable for REST API (please correct me if I'm wrong) while our contracts cover graphql queries and mutations.

@mefellows
Copy link
Member

Yes, currently bi-directional contracts only supports an OAS backed provider - GraphQL is in the plans, but for now we'll need to find another way forward.

@bethesque
Copy link
Member

I've done a proof of concept to aggregate all the interactions into a single uber-pact on the broker side. It needs a bit more work - the interactions have to be grouped by provider/consumer/WIP status/pact specification version, and then the results have to be spread back out into their respective source pacts, but I believe it's possible. Unfortunately, I don't know when I'm going to have time to do it, because I need to finish the branches/env support work before I pick up anything else. If you're interested, I can help you or someone with the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants