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

Fixed config locations to be URIs #22703

Merged
merged 1 commit into from
Jan 7, 2022

Conversation

mikethecalamity
Copy link

@mikethecalamity mikethecalamity commented Jan 6, 2022

The quarkus.config.locations configuration is specified as a java.net.URI in the documentation, but ConfigGenerationBuildStep was treating it as a file path

Fixes: #22699

@radcortez @glefloch @gsmet

@mikethecalamity
Copy link
Author

Can this be tagged to main and 2.6.2?

@quarkus-bot quarkus-bot bot added the area/core label Jan 6, 2022
@radcortez
Copy link
Member

Thanks for the PR :)

Can this be tagged to main and 2.6.2?

It will be added automatically once merged.

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 6, 2022

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

Failing Jobs - Building 8446636

Status Name Step Failures Logs Raw logs
Initial JDK 11 Build Build Failures Logs Raw logs

Failures

⚙️ Initial JDK 11 Build #

- Failing: core/deployment 
! Skipped: core/test-extension/deployment core/test-extension/runtime devtools/bom-descriptor-json and 607 more

📦 core/deployment

Failed to execute goal net.revelc.code:impsort-maven-plugin:1.6.2:check (check-imports) on project quarkus-core-deployment: Error reading file /home/runner/work/quarkus/quarkus/core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java

@radcortez
Copy link
Member

Ops, seems that the CI is not happy with the code style. Please do a maven build so the correct style is applied. Thank you!

@gsmet gsmet force-pushed the fix-config-location branch from 8446636 to 0ee58de Compare January 6, 2022 22:41
@gsmet
Copy link
Member

gsmet commented Jan 6, 2022

I'll fix the formatting issue but... I might be missing something.

We have in ConfigConfig (note the String):

    @ConfigItem(name = "config.locations")
    public Optional<List<String>> locations;

which is supposed to be a relocation for smallrye.config.locations so I'm unsure if reading this config property as URI would work? Or should it be URI everywhere?

Also the code has not been tested because it didn't compile (missing import and missing ;) - I fixed it too - so I'm totally unsure it fixes the reported issue?

@radcortez
Copy link
Member

I'll fix the formatting issue but... I might be missing something.

We have in ConfigConfig (note the String):

    @ConfigItem(name = "config.locations")
    public Optional<List<String>> locations;

which is supposed to be a relocation for smallrye.config.locations so I'm unsure if reading this config property as URI would work? Or should it be URI everywhere?

Actually, we only have ConfigConfig for documentation purposes. We don't use it, because this creates a chicken/egg problem since these configurations are used to configure the Config instance. We could update it to use URI anyway so it is correctly documented.

Also the code has not been tested because it didn't compile (missing import and missing ;) - I fixed it too - so I'm totally unsure it fixes the reported issue?

I was writing the exact same fix and I did test it in my Windows box.

@quarkus-bot
Copy link

quarkus-bot bot commented Jan 7, 2022

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

Failing Jobs - Building 0ee58de

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
Native Tests - Misc3 Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: core/test-extension/deployment integration-tests/smallrye-config 
! Skipped: integration-tests/test-extension 

📦 core/test-extension/deployment

io.quarkus.extest.AdditionalLocationsTest.additionalLocations - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

📦 integration-tests/smallrye-config

io.quarkus.it.smallrye.config.AppConfigMockTest.mockAppConfig - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.FallbackLocationsTest.fallback - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.HibernatePropertiesTest.properties - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.QuarkusTestProfileTest.testProfile - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

⚙️ JVM Tests - JDK 11 Windows #

- Failing: core/test-extension/deployment integration-tests/smallrye-config 
! Skipped: integration-tests/test-extension 

📦 core/test-extension/deployment

io.quarkus.extest.AdditionalLocationsTest.additionalLocations - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

📦 integration-tests/smallrye-config

io.quarkus.it.smallrye.config.AppConfigMockTest.mockAppConfig - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.FallbackLocationsTest.fallback - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.HibernatePropertiesTest.properties - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.QuarkusTestProfileTest.testProfile - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

⚙️ JVM Tests - JDK 17 #

- Failing: core/test-extension/deployment integration-tests/smallrye-config 
! Skipped: integration-tests/test-extension 

📦 core/test-extension/deployment

io.quarkus.extest.AdditionalLocationsTest.additionalLocations - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

📦 integration-tests/smallrye-config

io.quarkus.it.smallrye.config.AppConfigMockTest.mockAppConfig - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.FallbackLocationsTest.fallback - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.HibernatePropertiesTest.properties - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

io.quarkus.it.smallrye.config.QuarkusTestProfileTest.testProfile - More details - Source on GitHub

java.lang.RuntimeException: 
java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
	[error]: Build step io.quarkus.deployment.steps.ConfigGenerationBuildStep#watchConfigFiles threw an exception: java.lang.IllegalArgumentException: URI is not absolute

⚙️ Native Tests - Misc3 #

- Failing: integration-tests/smallrye-config 

📦 integration-tests/smallrye-config

Failed to execute goal io.quarkus:quarkus-maven-plugin:999-SNAPSHOT:build (default) on project quarkus-integration-test-smallrye-config: Failed to build quarkus application

@gsmet
Copy link
Member

gsmet commented Jan 7, 2022

CI doesn't look happy. @radcortez I'll let you have a look and check if this is fixable or not.

@radcortez
Copy link
Member

Ok, let me have a look.

@radcortez radcortez force-pushed the fix-config-location branch from 0ee58de to 5cea6c3 Compare January 7, 2022 14:17
@mikethecalamity
Copy link
Author

@radcortez Thanks for fixing it.

@gsmet
Copy link
Member

gsmet commented Jan 7, 2022

@radcortez I would prefer if we adjusted quarkus.config.locations to be a list of URI, even if it's not strictly necessary.

@radcortez
Copy link
Member

@radcortez I would prefer if we adjusted quarkus.config.locations to be a list of URI, even if it's not strictly necessary.

Sure.

@radcortez radcortez force-pushed the fix-config-location branch from 5cea6c3 to 52cb1cd Compare January 7, 2022 18:21
@gsmet gsmet added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 7, 2022
@radcortez radcortez merged commit b952bd6 into quarkusio:main Jan 7, 2022
@quarkus-bot quarkus-bot bot added this to the 2.7 - main milestone Jan 7, 2022
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 7, 2022
@gsmet gsmet modified the milestones: 2.7 - main, 2.6.2.Final Jan 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2.6 - Error with config locations
3 participants