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

Do not validate configs in unused beans #19965

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

radcortez
Copy link
Member

@radcortez radcortez commented Sep 7, 2021

This removes config validations and registrations of mappings / properties if the beans are unused. This may break existent applications if they only use programmatic lookups. In this case, mappings / properties need to be annotated with @Unremoveable or use the arc config to not remove beans.

@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Sep 7, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 7, 2021

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

Failing Jobs - Building 66e48fc

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 11 Windows Build Failures Logs Raw logs
✔️ JVM Tests - JDK 16

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/amazon-lambda/deployment 
! Skipped: docs extensions/amazon-lambda-http/deployment extensions/amazon-lambda-rest/deployment and 6 more

📦 extensions/amazon-lambda/deployment

io.quarkus.amazon.lambda.deployment.testing.LambdaDevServicesContinuousTestingTestCase.testLambda line 41 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <0> but was: <1>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)

Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

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

I've added one minor comment. Otherwise looks good!

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 8, 2021

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

Failing Jobs - Building 75043a5

Status Name Step Failures Logs Raw logs
✔️ JVM Tests - JDK 11
JVM Tests - JDK 16 Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 16 #

- Failing: extensions/smallrye-reactive-messaging-kafka/deployment 
! Skipped: docs integration-tests/kubernetes/quarkus-standard-way-kafka integration-tests/reactive-messaging-kafka and 1 more

📦 extensions/smallrye-reactive-messaging-kafka/deployment

io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingTestCase.testContinuousTestingScenario1 line 64 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: expected: <1> but was: <0>
	at org.junit.jupiter.api.AssertionUtils.fail(AssertionUtils.java:55)
	at org.junit.jupiter.api.AssertionUtils.failNotEqual(AssertionUtils.java:62)

@mikethecalamity
Copy link

Will this make it into 2.3?

@radcortez
Copy link
Member Author

Will this make it into 2.3?

Yes. I need to update this for something else and it should be good to go.

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 23, 2021

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

Failing Jobs - Building ea0adf2

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

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/hibernate-validator/deployment 
! Skipped: docs extensions/agroal/deployment extensions/apicurio-registry-avro/deployment and 187 more

📦 extensions/hibernate-validator/deployment

io.quarkus.hibernate.validator.test.config.ConfigMappingValidatorTest.validator line 31 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: validator.server.host must be less than or equal to 3 ==> Unexpected exception type thrown ==> expected: <io.smallrye.config.ConfigValidationException> but was: <java.util.NoSuchElementException>
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:65)
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:41)

⚙️ JVM Tests - JDK 11 Windows #

- Failing: extensions/hibernate-validator/deployment 
! Skipped: docs extensions/agroal/deployment extensions/apicurio-registry-avro/deployment and 187 more

📦 extensions/hibernate-validator/deployment

io.quarkus.hibernate.validator.test.config.ConfigMappingValidatorTest.validator line 31 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: validator.server.host must be less than or equal to 3 ==> Unexpected exception type thrown ==> expected: <io.smallrye.config.ConfigValidationException> but was: <java.util.NoSuchElementException>
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:65)
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:41)

⚙️ JVM Tests - JDK 17 #

- Failing: extensions/hibernate-validator/deployment 
! Skipped: docs extensions/agroal/deployment extensions/apicurio-registry-avro/deployment and 187 more

📦 extensions/hibernate-validator/deployment

io.quarkus.hibernate.validator.test.config.ConfigMappingValidatorTest.validator line 31 - More details - Source on GitHub

org.opentest4j.AssertionFailedError: validator.server.host must be less than or equal to 3 ==> Unexpected exception type thrown ==> expected: <io.smallrye.config.ConfigValidationException> but was: <java.util.NoSuchElementException>
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:65)
	at org.junit.jupiter.api.AssertThrows.assertThrows(AssertThrows.java:41)

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 23, 2021

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

Failing Jobs - Building 4b42c1f

Status Name Step Failures Logs Raw logs
Gradle 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.MultiModuleIncludedBuildTest.main line 24 - More details - Source on GitHub

java.lang.AssertionError: 

Expecting actual:

@radcortez radcortez requested a review from mkouba September 24, 2021 15:33
@mkouba
Copy link
Contributor

mkouba commented Sep 29, 2021

There are some conflicting files. @radcortez Could you resolve the conflicts first? I will need to check out the PR locally because the list of changes is quite long...

@radcortez
Copy link
Member Author

There are some conflicting files. @radcortez Could you resolve the conflicts first? I will need to check out the PR locally because the list of changes is quite long...

Done. The additional changes are to cover @ConfigMapping and @ConfigProperties usages.

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 29, 2021

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

Failing Jobs - Building 727920a

Status Name Step Failures Logs Raw logs
MicroProfile TCKs Tests Verify ⚠️ Check → Logs Raw logs

@radcortez
Copy link
Member Author

@mkouba thanks for the review. I've tried to address all the comments.

@radcortez radcortez requested a review from mkouba October 6, 2021 15:42
@quarkus-bot
Copy link

quarkus-bot bot commented Oct 6, 2021

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

Failing Jobs - Building 6378f80

Status Name Step Failures Logs Raw logs
Native Tests - Data1 Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Native Tests - Data1 #

- Failing: integration-tests/hibernate-orm-tenancy/connection-resolver-legacy-qualifiers 

📦 integration-tests/hibernate-orm-tenancy/connection-resolver-legacy-qualifiers

io.quarkus.it.hibernate.multitenancy.inventory.HibernateNamedPersistenceUnitTestInGraalITCase.testGetPlanesDefaultTenant - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code is <200> but was <500>.

io.quarkus.it.hibernate.multitenancy.inventory.HibernateNamedPersistenceUnitTestInGraalITCase.testGetPlanesTenantMycompany - More details - Source on GitHub

java.lang.AssertionError: 
1 expectation failed.
Expected status code is <200> but was <500>.

Copy link
Contributor

@mkouba mkouba 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.

@radcortez
Copy link
Member Author

Thanks! Sorry for the bumps!

@radcortez radcortez merged commit 0d75ec1 into quarkusio:main Oct 7, 2021
@quarkus-bot quarkus-bot bot added this to the 2.4 - main milestone Oct 7, 2021
@mikethecalamity
Copy link

@radcortez any chance this will be in 2.3.1?

@radcortez
Copy link
Member Author

Not likely, but 2.4.0 is not far. I'm sorry for the inconvenience :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/arc Issue related to ARC (dependency injection) area/config area/core area/hibernate-validator Hibernate Validator
Projects
None yet
3 participants