Removing oracle driver and associated tests

[fjtirado]

Removing quarkus-jdbc-oracle and disable all associated test because of legal issues with oracle driver

We also need to remove the driver from kogito-test-utils with runtimes PR https://github.com/apache/incubator-kie-kogito-runtimes/pull/3381

[fjtirado]

@porcelli I think you are misunderstanding the change. Build has to be disable (as the test) becuase oracle driver is being removed But once you setup an example with the driver (required) and quarkus build goal (which is done for all examples projects), the addon will still work. so the build removal is not changing anything besided disabling the test, it is just achieving whay you were trying to do with your PR (because that build is only needed for the quarkus IT phase)

[kie-ci3]

PR job #5 was: UNSTABLE Possible explanation: This should be test failures

Reproducer

build-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u https://github.com/apache/incubator-kie-kogito-apps/pull/1971 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution

Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-1971/5/display/redirect

Test results:

• PASSED: 2811
• FAILED: 1

Those are the test failures:

org.kie.kogito.index.ProcessDataIndexOracleIT.testProcessInstanceEvents

Failed to load ApplicationContext for [WebMergedContextConfiguration@67332b1e testClass = org.kie.kogito.index.ProcessDataIndexOracleIT, locations = [], classes = [org.kie.kogito.KogitoApplication], contextInitializerClasses = [org.kie.kogito.index.spring.KogitoServiceRandomPortSpringTestResource, org.kie.kogito.index.spring.DataIndexOracleSpringTestResource], activeProfiles = [], propertySourceLocations = [], propertySourceProperties = ["org.springframework.boot.test.context.SpringBootTestContextBootstrapper=true", "server.port=0"], contextCustomizers = [org.springframework.boot.test.context.filter.ExcludeFilterContextCustomizer@7905a0b8, org.springframework.boot.test.json.DuplicateJsonObjectContextCustomizerFactory$DuplicateJsonObjectContextCustomizer@6ca18a14, org.springframework.boot.test.mock.mockito.MockitoContextCustomizer@0, org.springframework.boot.test.web.client.TestRestTemplateContextCustomizer@6531a794, org.springframework.boot.test.autoconfigure.actuate.observability.ObservabilityContextCustomizerFactory$DisableObservabilityContextCustomizer@9da1, org.springframework.boot.test.autoconfigure.properties.PropertyMappingContextCustomizer@0, org.springframework.boot.test.autoconfigure.web.servlet.WebDriverContextCustomizerFactory$Customizer@3fce8fd9, org.springframework.boot.test.context.SpringBootTestAnnotation@11d7ecdf], resourceBasePath = "src/main/webapp", contextLoader = org.springframework.boot.test.context.SpringBootContextLoader, parent = null]

[davsclaus]

The newer Oracle JDBC drivers are released with FDHUT license https://download.oracle.com/otn-pub/otn_software/jdbc/FDHUT_LICENSE.txt?AuthParam=1706622347_ab29e2dc8dd1eb1814a13b41e7e8094f

These JDBC driver are easy to download and use from Maven central like any other JARs.

It may be good to get ASF legal to check this license.

[porcelli]

@davsclaus we have this issue with LEGAL, Justin McLean provided his perspective on the license there.

Spark member seems to have a different interpretation;

The KIE codebase seems to already depend on the latest Oracle JDBC driver.

[ricardozanini]

@porcelli I don't think we are infringing any legal aspects here. We are completely removing Oracle driver references and won't build or release this component. @fjtirado maybe we also have to remove it from the parent POM/BOM.

After 10.x we can move this code to a contrib repo and let it there be maintained by other members of the community, having their release pacing. Similar to what Quarkus Extensions do.

[fjtirado]

@porcelli As discusse Im working on a test refactor, moving the PR to draft

[fjtirado]

I uploaded a test refactor version that still have oracle. The next move is to change oracle by h2 (a common relational) and remove oracle container from runtimes. I guess we should also remove the oracle container, isnt it?

[porcelli]

yes, we need also remove container. expect to have another PR for the runtime repo that also have Oracle references.

[fjtirado]

yes, we need also remove container. expect to have another PR for the runtime repo that also have Oracle references.

yes. I have another PR on runtimes which I will be updating to to remove container.

[fjtirado]

@porcelli I have to fix breaking test, but more or less this is it We keep the generic jdbc code (from which a user can add oracle support to their project) tested and remove oracle driver and Oracle IT test that does not longer make sense.

[kie-ci3]

PR job #18 was: UNSTABLE Possible explanation: This should be test failures

Reproducer

build-chain build full_downstream -f 'https://raw.githubusercontent.com/${AUTHOR:apache}/incubator-kie-kogito-pipelines/${BRANCH:main}/.ci/buildchain-config-pr-cdb.yaml' -o 'bc' -p apache/incubator-kie-kogito-apps -u https://github.com/apache/incubator-kie-kogito-apps/pull/1971 --skipParallelCheckout NOTE: To install the build-chain tool, please refer to https://github.com/kiegroup/github-action-build-chain#local-execution

Please look here: https://ci-builds.apache.org/job/KIE/job/kogito/job/main/job/pullrequest_jobs/job/kogito-apps-pr/job/PR-1971/18/display/redirect

Test results:

• PASSED: 2788
• FAILED: 4

Those are the test failures:

org.kie.kogito.index.mongodb.ProcessDataIndexMongoDBHttpIT.testProcessInstanceEvents

1 expectation failed.
JSON path data.UserTaskInstances[0].potentialGroups[0] doesn't match.
Expected: managers
Actual: null

org.kie.kogito.index.mongodb.query.DomainQueryIT.test

java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
[error]: Build step io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor#startKeycloakContainer threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image quay.io/keycloak/keycloak:22.0.5
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.startKeycloakContainer(KeycloakDevServicesProcessor.java:248)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:864)
at io.quarkus.builder.BuildContext.run(BuildContext.java:282)
at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
at java.base/java.lang.Thread.run(Thread.java:833)
at org.jboss.threads.JBossThread.run(JBossThread.java:501)
Caused by: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image quay.io/keycloak/keycloak:22.0.5
at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:349)
at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:322)
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.lambda$startContainer$4(KeycloakDevServicesProcessor.java:374)
at java.base/java.util.Optional.orElseGet(Optional.java:364)
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.startContainer(KeycloakDevServicesProcessor.java:399)
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.startKeycloakContainer(KeycloakDevServicesProcessor.java:196)
... 11 more
Caused by: org.rnorth.ducttape.RetryCountExceededException: Retry limit hit with exception
at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:88)
at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:334)
... 16 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Could not create/start container
at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:553)
at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:344)
at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:81)
... 17 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for log output matching '.*Keycloak.*started.*'
at org.testcontainers.containers.wait.strategy.LogMessageWaitStrategy.waitUntilReady(LogMessageWaitStrategy.java:47)
at org.testcontainers.containers.wait.strategy.AbstractWaitStrategy.waitUntilReady(AbstractWaitStrategy.java:52)
at org.testcontainers.containers.GenericContainer.waitUntilContainerStarted(GenericContainer.java:964)
at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:490)
... 19 more

org.kie.kogito.index.postgresql.mapper.JobEntityMapperIT.testMapToModel

java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
[error]: Build step io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor#startKeycloakContainer threw an exception: java.lang.RuntimeException: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image quay.io/keycloak/keycloak:22.0.5
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.startKeycloakContainer(KeycloakDevServicesProcessor.java:248)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:568)
at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:864)
at io.quarkus.builder.BuildContext.run(BuildContext.java:282)
at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
at java.base/java.lang.Thread.run(Thread.java:833)
at org.jboss.threads.JBossThread.run(JBossThread.java:501)
Caused by: org.testcontainers.containers.ContainerLaunchException: Container startup failed for image quay.io/keycloak/keycloak:22.0.5
at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:349)
at org.testcontainers.containers.GenericContainer.start(GenericContainer.java:322)
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.lambda$startContainer$4(KeycloakDevServicesProcessor.java:374)
at java.base/java.util.Optional.orElseGet(Optional.java:364)
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.startContainer(KeycloakDevServicesProcessor.java:399)
at io.quarkus.oidc.deployment.devservices.keycloak.KeycloakDevServicesProcessor.startKeycloakContainer(KeycloakDevServicesProcessor.java:196)
... 11 more
Caused by: org.rnorth.ducttape.RetryCountExceededException: Retry limit hit with exception
at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:88)
at org.testcontainers.containers.GenericContainer.doStart(GenericContainer.java:334)
... 16 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Could not create/start container
at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:553)
at org.testcontainers.containers.GenericContainer.lambda$doStart$0(GenericContainer.java:344)
at org.rnorth.ducttape.unreliables.Unreliables.retryUntilSuccess(Unreliables.java:81)
... 17 more
Caused by: org.testcontainers.containers.ContainerLaunchException: Timed out waiting for log output matching '.*Keycloak.*started.*'
at org.testcontainers.containers.wait.strategy.LogMessageWaitStrategy.waitUntilReady(LogMessageWaitStrategy.java:47)
at org.testcontainers.containers.wait.strategy.AbstractWaitStrategy.waitUntilReady(AbstractWaitStrategy.java:52)
at org.testcontainers.containers.GenericContainer.waitUntilContainerStarted(GenericContainer.java:964)
at org.testcontainers.containers.GenericContainer.tryStart(GenericContainer.java:490)
... 19 more

PR check / Build projects / org.kie.kogito.addons.quarkus.data.index.it.PostgreSQLQuarkusAddonDataIndexPersistenceIT.testGraphQLUI

1 expectation failed.
Expected status code <404> but was <200>.

[elguardian]

@fjtirado so basically we move from oracle to jdbc (more general) and now we have jpa + jdbc implementation (two generic impl) ¿?

[fjtirado]

@fjtirado so basically we move from oracle to jdbc (more general) and now we have jpa + jdbc implementation (two generic impl) ¿?

We have just one jpa implementation and use that one to test postgres (and its non standard jsonb column) and generic jdbc (blob instead of jsonb), In future, I would like to explore the possibility of changing from jsonb to ElementColletion and do not divert because of that column, but this implies a schema change and it is a different discussion.

[fjtirado]

@porcelli Oracle package renamed to jdbc (h2 is a convenient way to test, should be working with any JPA compliant db), thanks for the pointer!

[fjtirado]

@elguardian Unfortunately I do not understand your concern, Which change are you requesting? Naming of the module? If so, which one do you consider right? The reason I named the module jdbc i because it will work with any jdbc compliant DB. I could have called it jsonasblob, but the name looks worse to me. As I already mention we should unify the handling of json column in future (the standard is really yo use an auxiliar table for map like fields). The JPA module should not directly be added as dependency, you will either add the "jdbc" one or the postgres one.

[elguardian]

Hi @fjtirado seems a bit weird to me. for instance

https://github.com/apache/incubator-kie-kogito-apps/pull/1971/files#diff-8016b6b06d81d9d9ff10d4a7039e71a34112c57e6fc4dc437c4ee6054b2988e2R61-R67

but if you are using the jdbc (which is really jpa based with some overriding) you are working with anything that is compliant with the jpa implementation but requires that overriding (not the entire jdbc)

so brifly

• we have a jpa (common that is not meant to be used by anybody and that could really work without anything else just defining the proper data source and adding the driver).. in fact there is no producer in here. I would suggests maybe for jpa-common.

• jdbc as a wrap of the jpa module plus overriding (the naming does not reflect what really is). I do think it is not really good name. I would say jpa-flavor being the flavor you are supporting. I can guarantee you that for pgsql big users will use bytea to avoid the vacuun utility (instead of oid or blobs...). so probably will have something like jpa-bytea or something like that. So keep in mind that better have this in mind now that start renaming things afterwards.

I think the strucutre allows some extension (probably not my preferred way) but it complies the need of some databases so it is fine. The thing doing the split (between jpa common and jdbc) it is only confusing as jpa is not meant to use by anybody. I would suggest jpa-common jpa-std (standard if the blob are meant to be supported by everyone....) jpa-byte... (or whatever flavours we ship.

The name does not reflect what really does, just calling jdbc because it seems we are using standard sql without the jpa middle man.

[fjtirado]

We have three modules:

• postgres, dependency to be added when using postgresql, that one needs to stay since changing that will be not backward compatibility
• jbbc, dependecy to be added when using a relational that is not posgresql, lets change to jpa. Why that instead of jpa-bytea? Because in future if we unify postgres and "rest of the dbs" json handling, we can still use the same dependency and do not need to add another
• jpa, which a module common to the two previous one that should not be added as dependency, lets change to jpa-common.

[fjtirado]

@elguardian Renaming done as per https://github.com/apache/incubator-kie-kogito-apps/pull/1971#issuecomment-1932114905