Skip to content

feat: Message Provider Implementation #200

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

Closed

Conversation

tuan-pham
Copy link
Contributor

@tuan-pham tuan-pham commented Jan 26, 2021

This is the second part of the Message Pact implementation. It covers the basic Message Provider verifier flow with generated pact file on local box.

  • Http Proxy is now generated by pact-python.
  • Basic flow to verify generated pact file on local box.

This is a DRAFT PR and not mean to be the final implementation. For the PR to be merged, the following bullet points need to be covered:

  • More unit tests for Message Provider and Http Proxy
  • Add support to verify provider with Pact Broker
  • Update Readme

Note:
Though I'm not 100% on Pact project, I can still come back and support this PR until it's merged to master. All feedback welcomed.

@tuan-pham tuan-pham changed the title Feature/message provider wip: Message Provider Implementation Jan 27, 2021
@tuan-pham tuan-pham changed the title wip: Message Provider Implementation feat: Message Provider Implementation Jan 27, 2021
@mefellows
Copy link
Member

Awesome Tuan!!

@elliottmurray elliottmurray marked this pull request as ready for review January 28, 2021 18:31
@elliottmurray elliottmurray marked this pull request as draft January 28, 2021 18:31
@@ -0,0 +1,91 @@
"""Http Proxy to be used as provider url in verifier."""
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the intention that Flask will run in the same process as out tests but in the background? Just beware you might have problems with later versions (Python and Flask) of this being a subprocess to do with tickle? Not a joke!

Copy link
Member

Choose a reason for hiding this comment

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

..ave problems with later versions (Python and Flask) of this being a subprocess to do with tickle? Not a joke!
😆

Is the intention that Flask will run in the same process as out tests but in the background

In case the context isn't clear. Basically, the way the provider verification for messages works still uses the same underlying verification binary, which expects to talk to an HTTP server. Instead of making the end user do the mapping, this proxy server is responsible for mapping the "message" to be verified, to a "function" that will produce the message - i.e. the actual provider.

See the sequence diagrams here for more: https://github.com/pact-foundation/pact-message-demo

If you knew all that already... sorry!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elliottmurray yes, the idea is to run it as a subprocess. There maybe a problem with later version with subprocess here but I'm not 100% sure what can we replace a subproces with.
@mefellows Thanks for reminding about sequence diagram. It helps understand the verify flow.

Copy link
Contributor

@elliottmurray elliottmurray Feb 2, 2021

Choose a reason for hiding this comment

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

I think you will need to use FastAPI over Flask @tuan-pham . I have an example in the examples. It is why I actually can't use the provider python in the e2e (which is Flask) and have to use a shell script instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mefellows I still don't understand this! I mean I get the provider side but the consumer side just makes my head hurt!

Choose a reason for hiding this comment

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

Hi I download @tuan-pham code. I'm using python 3.7 and lastest greatest flask. Which version of flask works? The issue I'm seeing is in the setup we set values on localstack. Then in the request we are getting None from the localstack. Not sure how to fix this. I figure different version of flask.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you will need to use FastAPI over Flask @tuan-pham . I have an example in the examples. It is why I actually can't use the provider python in the e2e (which is Flask) and have to use a shell script instead.

Hi @elliottmurray,
I'm working on the porting to FastAPI of the @tuan-pham work, any suggestions on how to replace werkzeug.local.LocalStack?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you will need to use FastAPI over Flask @tuan-pham . I have an example in the examples. It is why I actually can't use the provider python in the e2e (which is Flask) and have to use a shell script instead.

Hi @elliottmurray,
I'm working on the porting to FastAPI of the @tuan-pham work, any suggestions on how to replace werkzeug.local.LocalStack?

I've not looked at this recently due to personal stuff. I am going to start looking at moving to the underlying Rust impl and v3. However, I did do some work on this around messaging generally. I can't remember if I pulled Flask out on this branch or was just local:
https://github.com/pact-foundation/pact-python/commits/docs/kafka_example

The main issue IIRC was getting it to start up. But didn't get very far.

Copy link
Contributor Author

@tuan-pham tuan-pham Jul 15, 2021

Choose a reason for hiding this comment

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

@pulphix Yes, it's the plan to move to FastAPI over Flask. Unfortunately, it's not on my radar due to more higher priority assignment at my end. I may have a look into it at some point.

Choose a reason for hiding this comment

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

I setup maemcahce to solve the problem but fast api will solve this issue.

@@ -136,7 +136,7 @@ def with_content(self, contents):
:rtype: Pact
"""
self._insert_message_if_complete()
self._messages[0]['contents'] = contents
self._messages[0]['contents'] = from_term(contents)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, we need test for this!

@antonakospanos
Copy link

Any progress here, will this PR be merged? We'd use PACT on RMQ consumers too.

@mefellows
Copy link
Member

Is this PR defunct now that #251 was merged?

@elliottmurray
Copy link
Contributor

Is this PR defunct now that #251 was merged?

It is - I'll close it

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.

7 participants