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

Add support for new 'pacts for verification' endpoint #942

Closed
bethesque opened this issue Sep 26, 2019 · 12 comments
Closed

Add support for new 'pacts for verification' endpoint #942

bethesque opened this issue Sep 26, 2019 · 12 comments

Comments

@bethesque
Copy link
Member

bethesque commented Sep 26, 2019

See master issue here: pact-foundation/pact_broker#307

@bethesque
Copy link
Member Author

Copy paste from slack conversation:

Beth (Pactflow.io) 9:48 AM
Here's the issue, but the details are in the Pact Broker repo, because I need to open one issue in each of the repos, and didn't want to have to copy/paste all that text each time.

I've never worked on pact-jvm, so I'm afraid I can't give you much guidance, but you can ask in the pact-jvm channel.
You'll need to do some Kotlin, I believe.
Having done a poke around to find the place where the pacts are currently being fetched, I believe it's these methods:

https://github.com/DiUS/pact-jvm/blob/master/core/pact-broker/src/main/kotlin/au/com/dius/pact/core/pactbroker/PactBrokerClient.kt#L93
https://github.com/DiUS/pact-jvm/blob/master/core/pact-broker/src/main/kotlin/au/com/dius/pact/core/pactbroker/PactBrokerClient.kt#L70

I'm not sure why the class is called PactBrokerConsumer, when it appears to me to be a Pact object.
You'll need to find where those two methods are called, and turn those two separate calls into one single call to the new API.

data class PactBrokerConsumer @JvmOverloads constructor (
  val name: String,
  val source: String,
  val pactBrokerUrl: String,
  val pactFileAuthentication: List<String> = listOf(),
  val tag: String? = null
)

To me, the above class looks like a Pact, not a Consumer. It may need a refactor.

@araknoid
Copy link

Hi @bethesque ,
as discussed in the #994 , I'll work on this issue.
Please let me know any pointers/hints that could help me start on this issue.

@bethesque
Copy link
Member Author

My comment above is as much as I know in regards to the java code, sorry! Have a really good read of the master issue, and hop on to slack.pact.io and give me an @ to let me know you're there so we can discuss anything that's unclear. Ron should be able to give you pointers on the PactJVM code, but it would help if you had a poke around beforehand in your own IDE - I just worked out the stuff I've written in the comments by doing a few key word searches and following my nose through the code.

@uglyog
Copy link
Member

uglyog commented Jan 25, 2020

FYI

data class PactBrokerConsumer @JvmOverloads constructor ( val name: String, val source: String, val pactBrokerUrl: String, val pactFileAuthentication: List<String> = listOf(), val tag: String? = null )

To me, the above class looks like a Pact, not a Consumer. It may need a refactor.

That class does not represent a Pact (that is in core/model module), it represents the config for the consumer side of the relationship in the broker. When executed against the broker, it results in the URL of a Pact.

@bethesque
Copy link
Member Author

Oh, thanks Ron, that makes sense. It's a bit like what I've called a "selector" in the pact broker code and the docs for this issue then - a way to specify which pact(s) you want using some or all of consumer name/provider name/tags/scope, that later gets resolved to an actual pact url.

@bethesque
Copy link
Member Author

@araknoid are you still keen to do this? Ron was looking at picking it up but wasn't sure if you'd started

@araknoid
Copy link

araknoid commented Feb 1, 2020

Hi @bethesque , I started looking/exploring the code but didn't have enough time to actually do anything.
If someone else would like to take it, no problem (nothing will be lost 😉 ).
Said so, If Ron will take this one, is there one that I could be working on as I should have enough time from today onwards?
Thanks

@mefellows
Copy link
Member

I can see some commits against this card @uglyog , what's outstanding for it to be completed?

@uglyog
Copy link
Member

uglyog commented Apr 15, 2020

The base code is done, it just needs to be tested more

@sab1l
Copy link

sab1l commented May 28, 2020

Hi,
@uglyog, please remember to keep us posted regarding any progress here or expected release. We're looking forward to use this feature since we're facing the very issue it should solve.
Also we understand a proper testing may need time, so we'll wait whatever is needed.

@uglyog
Copy link
Member

uglyog commented May 28, 2020

This has been released with version 4.1.0

@bethesque
Copy link
Member Author

Closing as done! ❤️

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

5 participants