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.

Sign up for GitHub

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

Jump to bottom

KOGITO-5921 Support For OracleDB Peristence #1612

Merged
merged 20 commits into from
Oct 7, 2021
Merged

KOGITO-5921 Support For OracleDB Peristence #1612

merged 20 commits into from
Oct 7, 2021

Conversation

Scuilion
Copy link
Contributor

Add support for persisting to OracleDB
Just an initial stab to make sure that I'm on the right track. Will add test on follow up.
https://issues.redhat.com/browse/KOGITO-5921

Please make sure that your PR meets the following requirements:

  • You have read the contributors guide
  • Your code is properly formatted according to this configuration
  • Pull Request title is properly formatted: KOGITO-XYZ Subject
  • Pull Request title contains the target branch if not targeting main: [0.9.x] KOGITO-XYZ Subject
  • Pull Request contains link to the JIRA issue
  • Pull Request contains link to any dependent or related Pull Request
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket
How to retest this PR or trigger a specific build:

  • Run all builds
    Please add comment: Jenkins retest this

  • Run (or rerun) specific test(s)
    Please add comment: Jenkins (re)run [runtimes|optaplanner|apps|examples] tests

  • Quarkus LTS checks
    Please add comment: Jenkins run LTS

  • Run (or rerun) LTS specific test(s)
    Please add comment: Jenkins (re)run [runtimes|optaplanner|apps|examples] LTS

  • Native checks
    Please add comment: Jenkins run native

  • Run (or rerun) native specific test(s)
    Please add comment: Jenkins (re)run [runtimes|optaplanner|apps|examples] native

  • Full Kogito testing (with cloud images and operator BDD testing)
    Please add comment: Jenkins run BDD
    This check should be used only if a big change is done as it takes time to run, need resources and one full BDD tests check can be done at a time ...

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

26 similar comments
@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@kie-ci
Copy link
Contributor

kie-ci commented Sep 16, 2021

Can one of the admins verify this patch?

@danielezonca
Copy link
Contributor

@Scuilion
Can you please apply the changes included in https://github.com/kiegroup/kogito-runtimes/pull/1612#discussion_r716594439 ?
On Monday we have sprint cutoff and I would like to include this PR 👍

@Scuilion
Copy link
Contributor Author

@Scuilion Can you please apply the changes included in #1612 (comment) ? On Monday we have sprint cutoff and I would like to include this PR +1

commit 903d634 creates the following ddl zip

/home/kevino/projects/kogito-runtimes/addons/common/persistence ±[●][KOGITO-5921-support-for-oracle] k8sstaging/staging
> find . -name "*.zip"                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       
./ddl/target/kogito-ddl-runtimes-2.0.0-SNAPSHOT-db-scripts.zip                                                                                                                                                                                                                  
                                                                                                                                                                                                                                                                                
/home/kevino/projects/kogito-runtimes/addons/common/persistence ±[●][KOGITO-5921-support-for-oracle] k8sstaging/staging 
zipinfo ./ddl/target/kogito-ddl-runtimes-2.0.0-SNAPSHOT-db-scripts.zip                                                                                                                                                                                                       
Archive:  ./ddl/target/kogito-ddl-runtimes-2.0.0-SNAPSHOT-db-scripts.zip                                                                                                                                                                                                        
Zip file size: 1493 bytes, number of entries: 6                                                                                                                                                                                                                                 
drwxr-xr-x  2.0 unx        0 b- stor 21-Sep-30 13:09 postgresql/                                                                                                                                                                                                                
drwxr-xr-x  2.0 unx        0 b- stor 21-Sep-30 13:09 jdbc/                                                                                                                                                                                                                      
-rw-rw-r--  2.0 unx      467 bl defN 21-Sep-23 10:27 postgresql/runtime_create.sql                                                                                                                                                                                              
-rw-rw-r--  2.0 unx      285 bl defN 21-Sep-30 12:11 jdbc/create_tables_ansi.sql                                                                                                                                                                                                
-rw-rw-r--  2.0 unx      291 bl defN 21-Sep-30 12:11 jdbc/create_tables_PostgreSQL.sql                                                                                                                                                                                          
-rw-rw-r--  2.0 unx      286 bl defN 21-Sep-30 12:11 jdbc/create_tables_Oracle.sql                                                                                                                                                                                              
6 files, 1329 bytes uncompressed, 683 bytes compressed:  48.6%

@kie-ci
Copy link
Contributor

kie-ci commented Sep 30, 2021

The (build) Examples check has failed. Please check the logs.

@Scuilion
Copy link
Contributor Author

Can't seem to see the details of the failed Examples build.

@cristianonicolai
Copy link
Contributor

Jenkins rerun examples tests

@Scuilion
Copy link
Contributor Author

Scuilion commented Oct 1, 2021

woot woot!

@cristianonicolai
Copy link
Contributor

@Scuilion Can you please apply the changes included in #1612 (comment) ? On Monday we have sprint cutoff and I would like to include this PR +1

commit 903d634 creates the following ddl zip

/home/kevino/projects/kogito-runtimes/addons/common/persistence ±[●][KOGITO-5921-support-for-oracle] k8sstaging/staging
> find . -name "*.zip"                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                       
./ddl/target/kogito-ddl-runtimes-2.0.0-SNAPSHOT-db-scripts.zip                                                                                                                                                                                                                  
                                                                                                                                                                                                                                                                                
/home/kevino/projects/kogito-runtimes/addons/common/persistence ±[●][KOGITO-5921-support-for-oracle] k8sstaging/staging 
zipinfo ./ddl/target/kogito-ddl-runtimes-2.0.0-SNAPSHOT-db-scripts.zip                                                                                                                                                                                                       
Archive:  ./ddl/target/kogito-ddl-runtimes-2.0.0-SNAPSHOT-db-scripts.zip                                                                                                                                                                                                        
Zip file size: 1493 bytes, number of entries: 6                                                                                                                                                                                                                                 
drwxr-xr-x  2.0 unx        0 b- stor 21-Sep-30 13:09 postgresql/                                                                                                                                                                                                                
drwxr-xr-x  2.0 unx        0 b- stor 21-Sep-30 13:09 jdbc/                                                                                                                                                                                                                      
-rw-rw-r--  2.0 unx      467 bl defN 21-Sep-23 10:27 postgresql/runtime_create.sql                                                                                                                                                                                              
-rw-rw-r--  2.0 unx      285 bl defN 21-Sep-30 12:11 jdbc/create_tables_ansi.sql                                                                                                                                                                                                
-rw-rw-r--  2.0 unx      291 bl defN 21-Sep-30 12:11 jdbc/create_tables_PostgreSQL.sql                                                                                                                                                                                          
-rw-rw-r--  2.0 unx      286 bl defN 21-Sep-30 12:11 jdbc/create_tables_Oracle.sql                                                                                                                                                                                              
6 files, 1329 bytes uncompressed, 683 bytes compressed:  48.6%

@Scuilion thanks, its almost there, the idea is to keep a folder per database and jdbc would be the generic/ANSI version, so in that sense we need to endup with:

  • postgresql/runtime_create.sql
  • jdbc/runtime_create.sql
  • oracle/runtime_create.sql

Be aware that the Postgresql one is already there, so you just need to make sure that content matches.

@danielezonca
Copy link
Contributor

@Scuilion
I have applied my proposed changes directly to your branch, see kiegroup@3981a66

@cristianonicolai
This the output of the script now. Is it fine for you?
The PostgreSQL script is taken by postgres addon and not from the generic one. We might revisit this if necessary
image

@cristianonicolai
Copy link
Contributor

@danielezonca thanks!

MarianMacik
MarianMacik reviewed Oct 5, 2021
View reviewed changes
addons/common/persistence/jdbc/src/main/java/org/kie/kogito/persistence/jdbc/FileLoader.java Show resolved Hide resolved
kogito-build/kogito-dependencies-bom/pom.xml Show resolved Hide resolved
kogito-test-utils/src/main/java/org/kie/kogito/testcontainers/KogitoOracleSqlContainer.java Outdated
withLogConsumer(getLogger());
withLogConsumer(new Slf4jLogConsumer(LOGGER));
withStartupTimeout(Constants.CONTAINER_START_TIMEOUT);
System.setProperty("oracle.jdbc.timezoneAsRegion", "false");
Copy link
Member

Choose a reason for hiding this comment

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

+1 to that. For safety we should remove this flag after the tests are executed/container is stopped.

@kie-ci
Copy link
Contributor

kie-ci commented Oct 5, 2021

The (build) Examples check has failed. Please check the logs.

@kie-ci
Copy link
Contributor

kie-ci commented Oct 5, 2021

The (build) Runtimes check has failed. Please check the logs.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 5, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 8 Code Smells

0.0% 0.0% Coverage
2.5% 2.5% Duplication

@kie-ci
Copy link
Contributor

kie-ci commented Oct 5, 2021

The (build) Examples check has failed. Please check the logs.

@Scuilion
Copy link
Contributor Author

Scuilion commented Oct 5, 2021

Build failure details not available.

@danielezonca
Copy link
Contributor

Build failure details not available.

Jenkins build are less reliable compared to GHA so I have approved GHA to see if this is green. I expect it is a random (or unrelated) failure

@cristianonicolai
Copy link
Contributor

Jenkins rerun examples tests

@kie-ci
Copy link
Contributor

kie-ci commented Oct 6, 2021

The (build) Examples check has failed. Please check the logs.

@danielezonca
Copy link
Contributor

I think main branch of examples repo is broken, I got similar failures in this other PR https://github.com/kiegroup/kogito-runtimes/pull/1631

@evacchi
Can you please have a look? It seems related to new programmatic API examples even if it is not clear to me the relation 🤔

@cristianonicolai
Copy link
Contributor

Jenkins rerun examples tests

ricardozanini
ricardozanini reviewed Oct 6, 2021
View reviewed changes
addons/common/persistence/jdbc/src/test/java/org/kie/persistence/jdbc/TestHelper.java
@@ -66,12 +67,14 @@ public static void startContainerAndPublicPortIsAvailable() {

ORACLE_CONTAINER.start();
ORACLE_DATA_SOURCE = getOracleDataSource(ORACLE_CONTAINER);
System.setProperty(ORACLE_TIMEZONE_PROPERTY, "false");
Copy link
Member

Choose a reason for hiding this comment

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

I think it's fine to have this for now, but there's another solution for such use cases: https://www.baeldung.com/java-system-rules-junit#working-with-system-properties

@danielezonca danielezonca merged commit 4c0a54f into apache:main Oct 7, 2021
@danielezonca
Copy link
Contributor

@Scuilion
Thanks again for the PR, merged :)
It will be included in 1.12 release

@Scuilion Scuilion deleted the KOGITO-5921-support-for-oracle branch October 7, 2021 15:01
@debu999
Copy link

debu999 commented Dec 21, 2021

No documentation updated for oracle. Can that be done?? Only we have PostGres in documentation. Is it worth a look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiagodolphine tiagodolphine tiagodolphine left review comments

@ricardozanini ricardozanini ricardozanini approved these changes

@danielezonca danielezonca danielezonca approved these changes

@MarianMacik MarianMacik MarianMacik approved these changes

@cristianonicolai cristianonicolai cristianonicolai approved these changes

@evacchi evacchi Awaiting requested review from evacchi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@Scuilion @kie-ci @ricardozanini @danielezonca @cristianonicolai @debu999 @MarianMacik @tiagodolphine