Update REST client guides to use stage.code.quarkus.io instead of restcountries.eu #21166

xstefank · 2021-11-03T10:04:19Z

Fixes #20810

gsmet · 2021-11-03T11:25:36Z

docs/src/main/asciidoc/config-yaml.adoc

-    org.acme.restclient.CountriesService:
-      url: https://restcountries.eu/rest
+    org.acme.rest.client.ExtensionsService:
+      url: https://stage.code.quarkus.io/api


Hmmm, is it a good idea to use a staging instance?

this was requested - #20810 (comment)

I can see that :). But I'm not very convinced it's a good idea. @ia3andy don't you think it would be better to use the production API? I don't expect a ton of load and availability is important given you have people from all over the world testing this in different time zones.

staging is as available as production @gsmet

hmm, I don't see that much of a difference. A matter of preference? Let me know if I should change it.

staging is better, it has the same availability, we already have tests using it and most important of all at least it doesn't affect our usage statistics

OK, I'll keep my "I told you so" for later then ;).

Oh that's nasty :)

gsmet · 2021-11-04T10:24:03Z

@michalszynkiewicz can you have a quick look at that given it's about the REST Client?

And there's the corresponding quickstart PR here: quarkusio/quarkus-quickstarts#977 .

michalszynkiewicz · 2021-11-04T20:14:52Z

docs/src/main/asciidoc/native-and-ssl.adoc

 ----
 which configures our REST client to connect to an SSL REST service.

+For the purposes of this guide, we also need to remove the configuration that starts the embedded WireMock server that stubs REST client responses so the tests actually propagate calls to the https://stage.code.quarkus.io/api. Update the test file `src/test/java/org/acme/rest/client/ExtensionsResourceTest.java` and remove the line:


I don't get the motivation for removing wiremock

Wiremock server before this was wrong and propagated actual calls from tests to the restcountries.eu. This native SSL guide relied on this mistake which once fixed couldn't reproduce this issue that this guide was referring to as stubs worked :).

Maybe instead of showing the issue in tests, the guide could just mention that when someone tries it out, it fails?

Removing wiremock seems a bigger change to the project, and might send a message to not mock the API if it's https

Agreed, but I will do that in a subsequent PR. This is exactly why I didn't want to do broader refactoring under the "Update REST client guides..." commits :)

michalszynkiewicz · 2021-11-04T20:19:25Z

docs/src/main/asciidoc/native-and-ssl.adoc


 [source,bash]
 ----
-./mvnw clean install -Pnative
+./mvnw clean install -Pnative -DskipTests


if wiremock is not removed, there's no need to skip tests

So I have native SSL rework on my TODO list after this but didn't want to have unrelated changes in already bigger PR. The problem is that http://restcountries.eu/rest will respond with 302 Found which will fail the tests. Not sure if that should act as restcountries.eu where I suppose there was a redirect to https variant so the tests passed. A few possible solutions but I'm not sure which one would be the best. Should I adjust rest-client tests to expect either 200 and correct JSONs or 302 and no JSON?

please ping me on Zulip when you get to implement that :)

michalszynkiewicz · 2021-11-04T20:21:27Z

docs/src/main/asciidoc/rest-client-reactive.adoc

-====
-No `@PathParam` annotation is necessary on `countryName` as the name of the parameter matches the value used in the `@Path` annotation
-====
+The `getById` method gives our code the ability to query an extension by id from the Code Quarkus API. The client will handle all the networking and marshalling leaving our code clean of such technical details.


Maybe:

Suggested change

The `getById` method gives our code the ability to query an extension by id from the Code Quarkus API. The client will handle all the networking and marshalling leaving our code clean of such technical details.

The `getById` method gives our code the ability to get an extension by id from the Code Quarkus API.

The client will handle all the networking and marshalling leaving our code clean of such technical details.

michalszynkiewicz · 2021-11-04T20:27:26Z

docs/src/main/asciidoc/rest-client-reactive.adoc

-@Path("/v2")
-public interface CountriesService {
+@Path("/extensions")
+@RegisterRestClient


Suggested change

@RegisterRestClient

michalszynkiewicz · 2021-11-04T20:27:39Z

docs/src/main/asciidoc/rest-client-reactive.adoc

@@ -266,18 +252,19 @@ With this approach the client interface could look as follows:
 ----
 package org.acme.rest.client;

+import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;


Suggested change

import org.eclipse.microprofile.rest.client.inject.RegisterRestClient;

michalszynkiewicz · 2021-11-04T20:31:43Z

docs/src/main/asciidoc/rest-client-reactive.adoc

 }
 ----

 <1> There can be only one `ClientHeadersFactory` per class. With it, you can not only add custom headers, but you can also transform existing ones. See the `RequestUUIDHeaderFactory` class below for an example of the factory.
 <2> `@ClientHeaderParam` can be used on the client interface and on methods. It can specify a constant header value...
 <3> ... and a name of a method that should compute the value of the header. It can either be a static method or a default method in this interface
 <4> ... as well as a value from your application's configuration
+<5> ... or as a normal JAX-RS `@HeaderParam` annotated argument


good catch!

michalszynkiewicz · 2021-11-04T20:37:56Z

thanks for handling it! The only big issue is the @RegisterRestClient annotation added in the programmatic rest client creation.

michalszynkiewicz · 2021-11-04T20:38:30Z

By the way, can you squash the commits?

xstefank · 2021-11-05T13:33:11Z

@michalszynkiewicz thanks for the review. Do you think that squashing is better? Touching multiple QS so I thought it better to split but surely I can squash just please confirm.

Native SSL guide will probably need a broader rework which I can do in later PR.

michalszynkiewicz · 2021-11-05T21:29:44Z

LGTM, could you squash the commits?

… of https://restcountries.eu Update native-and-ssl guide to work with https://stage.code.quarkus.io/api Update REST Client reactive guide to work with https://stage.code.quarkus.io/api Update config-yaml guide to work with https://stage.code.quarkus.io/api

geoand · 2021-11-09T07:05:41Z

~~@xstefank can you squash the commits so we can get this in?~~

Nevermind, just saw you did :)

quarkus-bot bot added the area/documentation label Nov 3, 2021

xstefank mentioned this pull request Nov 3, 2021

Update REST client quickstarts to use stage.code.quarkus.io instead of restcountries.eu quarkusio/quarkus-quickstarts#977

Merged

7 tasks

gsmet reviewed Nov 3, 2021

View reviewed changes

michalszynkiewicz reviewed Nov 4, 2021

View reviewed changes

xstefank force-pushed the issue-20810 branch from ecc8b91 to 1177128 Compare November 5, 2021 13:32

xstefank force-pushed the issue-20810 branch from 1177128 to 3dccf81 Compare November 8, 2021 09:33

geoand requested a review from michalszynkiewicz November 9, 2021 07:06

michalszynkiewicz approved these changes Nov 9, 2021

View reviewed changes

michalszynkiewicz merged commit 1011581 into quarkusio:main Nov 9, 2021

quarkus-bot bot added this to the 2.5 - main milestone Nov 9, 2021

gsmet mentioned this pull request Mar 24, 2022

Disable QuarkusCodestartBuildIT for now #24544

Merged

ia3andy mentioned this pull request Mar 25, 2022

Add inject example as a comment & rm the tests in rest-client codestart #24551

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update REST client guides to use stage.code.quarkus.io instead of restcountries.eu #21166

Update REST client guides to use stage.code.quarkus.io instead of restcountries.eu #21166

xstefank commented Nov 3, 2021 •

edited

Loading

gsmet Nov 3, 2021

xstefank Nov 3, 2021

gsmet Nov 3, 2021

ia3andy Nov 3, 2021

xstefank Nov 3, 2021

ia3andy Nov 3, 2021 •

edited

Loading

gsmet Nov 4, 2021

ia3andy Nov 4, 2021

gsmet commented Nov 4, 2021

michalszynkiewicz Nov 4, 2021

xstefank Nov 5, 2021

michalszynkiewicz Nov 5, 2021

xstefank Nov 5, 2021

michalszynkiewicz Nov 4, 2021

xstefank Nov 5, 2021

michalszynkiewicz Nov 5, 2021

michalszynkiewicz Nov 4, 2021 •

edited

Loading

michalszynkiewicz Nov 4, 2021

michalszynkiewicz Nov 4, 2021

michalszynkiewicz Nov 4, 2021

michalszynkiewicz commented Nov 4, 2021

michalszynkiewicz commented Nov 4, 2021

xstefank commented Nov 5, 2021

michalszynkiewicz commented Nov 5, 2021

geoand commented Nov 9, 2021 •

edited

Loading

Update REST client guides to use stage.code.quarkus.io instead of restcountries.eu #21166

Update REST client guides to use stage.code.quarkus.io instead of restcountries.eu #21166

Conversation

xstefank commented Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ia3andy Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gsmet commented Nov 4, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalszynkiewicz Nov 4, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalszynkiewicz commented Nov 4, 2021

michalszynkiewicz commented Nov 4, 2021

xstefank commented Nov 5, 2021

michalszynkiewicz commented Nov 5, 2021

geoand commented Nov 9, 2021 • edited Loading

xstefank commented Nov 3, 2021 •

edited

Loading

ia3andy Nov 3, 2021 •

edited

Loading

michalszynkiewicz Nov 4, 2021 •

edited

Loading

geoand commented Nov 9, 2021 •

edited

Loading