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

Rest client config - turn the config module into a proper extension #21788

Merged

Conversation

TomasHofman
Copy link
Contributor

This is needed for the automatic docs generation to work properly.

Fixes #21488

@quarkus-bot quarkus-bot bot added area/dependencies Pull requests that update a dependency file area/rest labels Nov 29, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 29, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building acf9748

Status Name Step Failures Logs Raw logs
Initial JDK 11 Build Verify extension dependencies ⚠️ Check → Logs Raw logs

@geoand
Copy link
Contributor

geoand commented Nov 29, 2021

Needs a dependencies update - see the error message in the logs

This is needed for the automatic docs generation to work properly.
@TomasHofman TomasHofman force-pushed the rest-client-config-generated-docs branch from acf9748 to 9bba11d Compare November 29, 2021 15:47
@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation labels Nov 29, 2021
@gsmet
Copy link
Member

gsmet commented Nov 29, 2021

Hmmm I haven't followed all this but I find it weird we have to introduce an extension just for some config classes.

Could you summarize to me why we ended up with a separate module for the config?

Thanks!

@quarkus-bot
Copy link

quarkus-bot bot commented Nov 29, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 9bba11d

Status Name Step Failures Logs Raw logs
Gradle Tests - JDK 11 Windows Build Failures Logs Raw logs
Maven Tests - JDK 11 Windows Build Failures Logs Raw logs
MicroProfile TCKs Tests Verify ⚠️ Check → Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Gradle Tests - JDK 11 Windows #

- Failing: integration-tests/gradle 

📦 integration-tests/gradle

io.quarkus.gradle.devmode.MultiModuleNamedInjectionDevModeTest.main line 21 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

⚙️ Maven Tests - JDK 11 Windows #

- Failing: integration-tests/maven 

📦 integration-tests/maven

io.quarkus.maven.it.DevMojoIT.testThatSourceChangesAreDetectedOnPomChange line 423 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.DevMojoIT that uses java.lang.String was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

io.quarkus.maven.it.DevMojoIT.testThatSourceChangesAreDetectedOnPomChange line 423 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.maven.it.DevMojoIT that uses java.lang.String was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

@TomasHofman
Copy link
Contributor Author

Hello @gsmet, so one aspect is that we want to share configuration classes between classic and reactive rest clients - that's why it's in a separate module.

Then we found that the "quarkus.rest-client.*" configuration properties that we define in our config roots are missing from auto-generated docs (https://quarkus.io/guides/all-config). The reason is that the generator class (AllConfigGenerator) only detects config roots if they are part of extension jars directly (or that's how it appears to me).

@quarkus-bot
Copy link

quarkus-bot bot commented Dec 1, 2021

This workflow status is outdated as a new workflow run has been triggered.

Failing Jobs - Building 9bba11d

Status Name Step Failures Logs Raw logs
Gradle Tests - JDK 11 Windows Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Gradle Tests - JDK 11 Windows #

- Failing: integration-tests/gradle 

📦 integration-tests/gradle

io.quarkus.gradle.devmode.QuarkusDevDependencyDevModeTest.main line 14 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:164)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Looks good to me. Let's merge it to have it for CR1. Thanks for the follow-up!

@gsmet gsmet merged commit ad4ab21 into quarkusio:main Dec 7, 2021
@quarkus-bot quarkus-bot bot added this to the 2.6 - main milestone Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation area/rest
Projects
None yet
Development

Successfully merging this pull request may close these issues.

quarkus.rest-client.* properties not present in all-config guide
3 participants