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

feat: added support for message provider using pact broker #257

Conversation

pulphix
Copy link
Contributor

@pulphix pulphix commented Aug 28, 2021

Added Pact Broker Support for Message Provider Issue #253

@elliottmurray
Copy link
Contributor

This is looking great again! Just one question around the examples tests. I'll need to hook these into CI I think as well. I think I have an example that uses test containers you might want to consider btw.

fix: updated message_pact to wait writing contract process to finish
@pulphix
Copy link
Contributor Author

pulphix commented Aug 30, 2021

This is looking great again! Just one question around the examples tests. I'll need to hook these into CI I think as well. I think I have an example that uses test containers you might want to consider btw.

I did change the examples for messages to use a broker docker instance.
In some Etiqa projects, we are using https://github.com/tox-dev/tox-docker
This allows tox to run a docker and run tests against it.

Copy link

@arturoriter arturoriter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used this and worked like a charm... anxiously waiting for it to be merged.

},
provider='ContentProvider',
consumer='DetectContentLambda',
pact_dir='pacts'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the pact_dir should not be here anymore and works just fine if you remove it.

'A document created successfully': document_created_handler,
},
provider='ContentProvider',
consumer='DetectContentLambda',

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the consumer should no longer be mandatory because does not make sense since you fetch the data from the pact broker

@elliottmurray
Copy link
Contributor

Sorry - I've been busy and then sick this week. I'll hopefully get to it this weekend.

@elliottmurray elliottmurray merged commit 08f0dc0 into pact-foundation:master Sep 5, 2021
@@ -115,6 +115,25 @@ def verify(self):
return_code, _ = verifier.verify_pacts(pact_files, verbose=False)
assert (return_code == 0), f'Expected returned_code = 0, actual = {return_code}'

def verify_with_broker(self, enable_pending=False, include_wip_pacts_since=None, **kwargs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this user facing code? It's missing the consumer version selectors. How can we get those supported?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Successfully merging this pull request may close these issues.

None yet

4 participants