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

Fix service binding for SqlServer and Oracle #27467

Merged
merged 1 commit into from
Aug 24, 2022

Conversation

xieshenzh
Copy link
Contributor

fixes #27461

  1. Update the connection url for Oracle and SqlServer that is generated by the kubernetes-service-binding extension.
  2. Add a new service binding configuration option jdbc-url to allow user to customize connection url for service binding. This is similar to what spring-service-binding supports.

@xieshenzh xieshenzh force-pushed the service-binding-sqlserver-oracle branch from 8ad50b1 to 9743d7e Compare August 24, 2022 03:20
@xieshenzh xieshenzh marked this pull request as ready for review August 24, 2022 04:01
Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

Thanks!

@geoand
Copy link
Contributor

geoand commented Aug 24, 2022

@gsmet does the format also look good to you?

@quarkus-bot
Copy link

quarkus-bot bot commented Aug 24, 2022

Failing Jobs - Building 9743d7e

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

Full information is available in the Build summary check run.

Failures

⚙️ JVM Tests - JDK 11 #

- Failing: extensions/smallrye-reactive-messaging-kafka/deployment 
! Skipped: integration-tests/kafka-oauth-keycloak integration-tests/kafka-sasl-elytron integration-tests/kubernetes/quarkus-standard-way-kafka and 3 more

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

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

org.awaitility.core.ConditionTimeoutException: Failed to wait for test run 4 State{lastRun=3, running=true, inProgress=true, run=1, passed=0, failed=1, skipped=0, isBrokenOnly=false, isTestOutput=false, isInstrumentationBasedReload=false, isLiveReload=true}
	at io.quarkus.test.ContinuousTestingTestUtils.waitForNextCompletion(ContinuousTestingTestUtils.java:44)
	at io.quarkus.smallrye.reactivemessaging.kafka.deployment.testing.KafkaDevServicesContinuousTestingTestCase.testContinuousTestingScenario2(KafkaDevServicesContinuousTestingTestCase.java:83)

@@ -13,6 +13,6 @@ public class MsSQLServiceBindingConverter implements ServiceBindingConverter {
@Override
public Optional<ServiceBindingConfigSource> convert(List<ServiceBinding> serviceBindings) {
return ServiceBinding.singleMatchingByType("sqlserver", serviceBindings)
.map(new DatasourceServiceBindingConfigSourceFactory.Jdbc());
.map(new DatasourceServiceBindingConfigSourceFactory.Jdbc("jdbc:%s://%s%s;databaseName=%s"));
Copy link
Member

Choose a reason for hiding this comment

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

Not related to your patch but this is really brittle and close to unreadable. But let's not slow down your patch because of that as it's preexisting.

I would rather have to implement a method that takes proper parameters with names.

@gsmet gsmet merged commit 85d06ba into quarkusio:main Aug 24, 2022
@quarkus-bot quarkus-bot bot added this to the 2.13 - main milestone Aug 24, 2022
@gsmet gsmet modified the milestones: 2.13 - main, 2.12.1.Final Sep 5, 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.

Service binding doesn't work with Oracle and SqlServer
3 participants