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

Replaced deprecated Config phase BOOTSTRAP in Kubernetes #34457

Merged
merged 2 commits into from
Jul 4, 2023

Conversation

radcortez
Copy link
Member

No description provided.

@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Jul 3, 2023

@radcortez any chance you break up the change to the kubernete-client and kubernetes-config modules into separate commits?

@radcortez
Copy link
Member Author

Separate commits in the same PR you mean? Or do you prefer to have separate PRs?

@geoand
Copy link
Contributor

geoand commented Jul 3, 2023

Same PR is fine

@radcortez
Copy link
Member Author

Ok, let me see what I can do.

@quarkus-bot

This comment has been minimized.

@geoand
Copy link
Contributor

geoand commented Jul 3, 2023

Sorry for nitpicking, but could you fix the description of the commit for the kubernetes-client, as the commit just changes to ConfigMapping (the is no Bootstrap phase for the kubernetes-client) extension.

@geoand
Copy link
Contributor

geoand commented Jul 3, 2023

Also cc, @metacosm as this might break the JOSDK

@radcortez
Copy link
Member Author

Sorry for nitpicking, but could you fix the description of the commit for the kubernetes-client, as the commit just changes to ConfigMapping (the is no Bootstrap phase for the kubernetes-client) extension.

Sure. No worries.

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 3, 2023

Failing Jobs - Building 6f6fb9e

Status Name Step Failures Logs Raw logs
✔️ Gradle Tests - JDK 11
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.builder.QuarkusModelBuilderTest.shouldLoadMultiModuleTestModel line 66 - More details - Source on GitHub

org.gradle.tooling.BuildException: Could not run build action using connection to Gradle distribution 'https://services.gradle.org/distributions/gradle-8.1.1-bin.zip'.
	at org.gradle.tooling.internal.consumer.ExceptionTransformer.transform(ExceptionTransformer.java:51)
	at org.gradle.tooling.internal.consumer.ExceptionTransformer.transform(ExceptionTransformer.java:29)

io.quarkus.gradle.builder.QuarkusModelBuilderTest.shouldLoadMultiModuleDevModel line 100 - More details - Source on GitHub

org.gradle.tooling.BuildException: Could not run build action using connection to Gradle distribution 'https://services.gradle.org/distributions/gradle-8.1.1-bin.zip'.
	at org.gradle.tooling.internal.consumer.ExceptionTransformer.transform(ExceptionTransformer.java:51)
	at org.gradle.tooling.internal.consumer.ExceptionTransformer.transform(ExceptionTransformer.java:29)

@metacosm
Copy link
Contributor

metacosm commented Jul 4, 2023

Can I get a TL,DR; description of what this does and what purpose this serves, as well to how things are potentially affected downstream, please?

@geoand
Copy link
Contributor

geoand commented Jul 4, 2023

What you care about is that KubernetesClientBuildConfig and KubernetesDevServicesBuildTimeConfig are now interfaces

@metacosm
Copy link
Contributor

metacosm commented Jul 4, 2023

Ok, where could I learn more about why this change was made? In particular, is it something that people are now expected to do in their own extensions (as opposed to having public fields on config classes)?

@geoand
Copy link
Contributor

geoand commented Jul 4, 2023

Ok, where could I learn more about why this change was made?

@radcortez ^

In particular, is it something that people are now expected to do in their own extensions (as opposed to having public fields on config classes)?

Yes, but there is no rush.

@radcortez
Copy link
Member Author

Ok, where could I learn more about why this change was made?

Certainly. Here is something that I wrote about the Class vs Interface approach:
jakartaee/config#122

Also, there are some additional reasons for Quarkus:

  • The old class / ConfigRoot approach works exclusively for Quarkus extensions, meaning that application developers had to use a different mechanism. The new interface approach works for both extensions and applications
  • The old mechanism was built around the Config system, which complicated a few things (for instance, the config classes would only be available at a certain point in the runtime). The new system is built inside the Config system, so they are immediately available once Config is up.

@radcortez radcortez merged commit 3734e34 into quarkusio:main Jul 4, 2023
33 of 34 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.3 - main milestone Jul 4, 2023
@metacosm
Copy link
Contributor

metacosm commented Jul 4, 2023

Thanks for the info, @radcortez!

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.

None yet

3 participants