Skip to content

Conversation

@jimma
Copy link
Contributor

@jimma jimma commented Oct 21, 2022

…nder microprofile-client module

@jimma jimma requested a review from reta October 21, 2022 12:38
@jimma
Copy link
Contributor Author

jimma commented Oct 21, 2022

@reta It seems we pull the com.github.tomakehurst:wiremock-standalone:2.27.2 test scope dependency and it works for all tests under rt/rs/microprofile-client and systests/jaxrs/JAXRSClientMetricsTest. Although this imports some javax servelt and jetty9 artifacts , but they are all for test purpose and doesn't pollute any microprofile client and system test client side dependencies. After the wiremock jakarta version is ready ,we can change back to use the com.github.tomakehurst:wiremock dependency to imports the jakarta servelt and jetty11 dependency. If you want me to include wiremock change for system/jaxrs, please let me know.

@reta
Copy link
Member

reta commented Oct 22, 2022

@reta It seems we pull the com.github.tomakehurst:wiremock-standalone:2.27.2 test scope dependency and it works for all tests under rt/rs/microprofile-client and systests/jaxrs/JAXRSClientMetricsTest. Although this imports some javax servelt and jetty9 artifacts , but they are all for test purpose and doesn't pollute any microprofile client and system test client side dependencies. After the wiremock jakarta version is ready ,we can change back to use the com.github.tomakehurst:wiremock dependency to imports the jakarta servelt and jetty11 dependency. If you want me to include wiremock change for system/jaxrs, please let me know.

This is useful trick, thanks @jimma ! Do you think we could wait a bit before merging this one (🤞 the we would have wiremock snapshots available)? For system/jaxrs I was thinking to phase wiremock out (it is used in very limited scope), working on that.

@jimma
Copy link
Contributor Author

jimma commented Oct 26, 2022

This is useful trick, thanks @jimma ! Do you think we could wait a bit before merging this one (🤞 the we would have wiremock snapshots available)?

@reta I'd prefer to add this to upstream to show how many jakarta failures left we need to fix before we have a wiremock(with jetty11 update) snapshot available. Because update to wiremock snapshot could be one line change , and it's easy to change after it's available. WDYT ?

For system/jaxrs I was thinking to phase wiremock out (it is used in very limited scope), working on that.
That's very cool. Thanks @reta !

@reta
Copy link
Member

reta commented Oct 26, 2022

This is useful trick, thanks @jimma ! Do you think we could wait a bit before merging this one (crossed_fingers the we would have wiremock snapshots available)?

@reta I'd prefer to add this to upstream to show how many jakarta failures left we need to fix before we have a wiremock(with jetty11 update) snapshot available. Because update to wiremock snapshot could be one line change , and it's easy to change after it's available. WDYT ?

For system/jaxrs I was thinking to phase wiremock out (it is used in very limited scope), working on that.
That's very cool. Thanks @reta !

@jimma sure, no objections, could you please rebase / merge with main? thank you.

@jimma
Copy link
Contributor Author

jimma commented Oct 27, 2022

Done. Thanks @reta.

@jimma jimma merged commit d31e1a3 into apache:main Oct 27, 2022
@reta
Copy link
Member

reta commented Oct 27, 2022

Done. Thanks @reta.

Thanks @jimma but I don't see it making the difference:

java.lang.NoClassDefFoundError: javax/servlet/DispatcherType
	at java.base/java.lang.Class.forName0(Native Method)
	at java.base/java.lang.Class.forName(Class.java:375)

Do you?

@jimma
Copy link
Contributor Author

jimma commented Oct 31, 2022

@reta It looks I missed another commit and sent it with another PR : #1018.

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.

2 participants