#11192 - Tck tests and initial support for MySQL by BarryDrez · Pull Request #11315 · eclipse-che/che

BarryDrez · 2018-09-21T15:51:31Z

What does this PR do?

This PR adds Tck tests for MySQL along with initial support. Multiuser tests for MySQL to be added soon.

What issues does this PR fix or reference?

#11192

Provide Initial support and tck tests for MySQL

Release Notes

Initial support for MySQL

Docs PR

riuvshin · 2018-09-21T15:51:32Z

Can one of the admins verify this patch?

riuvshin · 2018-09-21T15:51:32Z

Can one of the admins verify this patch?

benoitf · 2018-09-21T16:05:57Z

ci-build

Signed-off-by: Barry Dresdner <barry.dresdner@softwareag.com>

BarryDrez · 2018-09-21T16:52:47Z

@riuvshin, I added a second commit with a sign-off footer. I don't think I have permission to merge the commits.

skabashnyuk · 2018-09-24T06:13:53Z

ci-test

riuvshin · 2018-09-24T06:34:13Z

Results of automated E2E tests of Eclipse Che Multiuser on OCP:
Build details
Test report
docker image: eclipseche/che-server:11315
https://github.com/orgs/eclipse/teams/eclipse-che-qa please check this report.

mshaposhnik · 2018-09-24T07:46:20Z

...rt/src/main/resources/che-schema111/5.0.0-M9/mysql/2__update_local_links_in_environments.sql

@@ -0,0 +1,14 @@
+--


What is the purpose of che-schema111 set ?

@mshaposhnik Thanks for catching this. I meant to remove it.

mshaposhnik · 2018-09-24T07:52:51Z

core/che-core-db-vendor-mysql/pom.xml

+        <version>6.12.0-SNAPSHOT</version>
+    </parent>
+    <artifactId>che-core-db-vendor-mysql</artifactId>
+    <name>Che Core :: DB :: Vendor PostgreSQL</name>


Vendor MySQL

mshaposhnik · 2018-09-24T09:43:20Z

wsmaster/integration-tests/mysql-tck/pom.xml

+            <dependency>
+                <groupId>mysql</groupId>
+                <artifactId>mysql-connector-java</artifactId>
+                <version>8.0.12</version>


That needs to be CQ-ed and moved into che-parent (we can do that btw)

https://dev.eclipse.org/ipzilla/show_bug.cgi?id=17680

mshaposhnik · 2018-09-24T10:38:40Z

wsmaster/integration-tests/mysql-tck/pom.xml

+        <module>mysql-support</module>
+    </modules>
+    <properties>
+        <db.image.name>centos/mysql-57-centos7</db.image.name>


Not sure if we need that here

mkuznyetsov · 2018-09-24T11:12:23Z

...mysql/src/main/java/org/eclipse/che/core/db/mysql/jpa/eclipselink/MySqlExceptionHandler.java

+ * Contributors:
+ *   Red Hat, Inc. - initial API and implementation
+ */
+


please add package definition (under license header)

skabashnyuk · 2018-09-24T11:21:34Z

@BarryDrez can you make sure that you can compile your code with mvn clean install?

reviews. Signed-off-by: Barry Dresdner <barry.dresdner@softwareag.com>

BarryDrez · 2018-09-24T13:27:48Z

@BarryDrez can you make sure that you can compile your code with mvn clean install?
@skabashnyuk - It has been compiling even without the import, but that may have been because I had the error handler in the same package. I later moved it to a separate project. I have just made some corrections based on the peer reviews, and I committed them to my fork.

mshaposhnik · 2018-09-24T13:29:24Z

@BarryDrez Barry. Is it ok for you if we merge this PR into separate Che branch and finish all the required dependency stuff there (it will be faster if we do those fixes ourserves)?

skabashnyuk · 2018-09-24T13:30:42Z

@BarryDrez I see quite a similar modules

wsmaster/integration-tests/mysql-tck/pom.xml
wsmaster/integration-tests/mysql-tck/mysql-tck/pom.xml
can you take a look?

BarryDrez · 2018-09-24T13:34:53Z

@BarryDrez I see quite a similar modules

wsmaster/integration-tests/mysql-tck/pom.xml
wsmaster/integration-tests/mysql-tck/mysql-tck/pom.xml
can you take a look?
@skabashnyuk wsmaster/integration-tests/mysql-tck/pom.xml is the parent project, and wsmaster/integration-tests/mysql-tck/mysql-tck/pom.xml runs the actual tests.
The projects have different names mysql and mysql-tck. Is that OK, or should the directory names be different?

BarryDrez · 2018-09-24T13:35:40Z

@BarryDrez Barry. Is it ok for you if we merge this PR into separate Che branch and finish all the required dependency stuff there (it will be faster if we do those fixes ourserves)?
@mshaposhnik, sure, that will be fine. Hopefully after I get the remaining issues resolved.

mshaposhnik · 2018-09-24T13:40:31Z

Ok, fine. When ready, change base of this PR to https://github.com/eclipse/che/tree/mysql_support branch and merge. We'll do the rest of dependency stuff and merge to master than.

from peer review.

BarryDrez · 2018-09-24T16:19:02Z

@mshaposhnik, @skabashnyuk I have committed the changes as suggested.

#11192 - Tck tests and initial support for MySQL

5355c79

BarryDrez requested review from riuvshin, skabashnyuk and vparfonov as code owners September 21, 2018 15:51

benoitf added status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. kind/task Internal things, technical debt, and to-do tasks to be performed. labels Sep 21, 2018

#11192 - Tck tests and initial support for MySQL

04dc558

Signed-off-by: Barry Dresdner <barry.dresdner@softwareag.com>

mshaposhnik reviewed Sep 24, 2018

View reviewed changes

mkuznyetsov reviewed Sep 24, 2018

View reviewed changes

BarryDrez added 2 commits September 24, 2018 07:34

Merge branch 'master' of https://github.com/eclipse/che

6457434

#11192 - Tck and initial support for MySQL. Corrections from peer

cc733c0

reviews. Signed-off-by: Barry Dresdner <barry.dresdner@softwareag.com>

#11192 - Tck tests and initial support for MySQL. Incorporated feedback

e085fba

from peer review.

mshaposhnik changed the base branch from master to mysql_support September 25, 2018 06:39

mshaposhnik merged commit 66c0fc3 into eclipse-che:mysql_support Sep 25, 2018

benoitf added target/branch Indicates that a PR will be merged into a branch other than master. and removed status/code-review This issue has a pull request posted for it and is awaiting code review completion by the community. labels Sep 25, 2018

Conversation

BarryDrez commented Sep 21, 2018

What does this PR do?

What issues does this PR fix or reference?

Release Notes

Docs PR

Uh oh!

riuvshin commented Sep 21, 2018

Uh oh!

riuvshin commented Sep 21, 2018

Uh oh!

benoitf commented Sep 21, 2018

Uh oh!

BarryDrez commented Sep 21, 2018

Uh oh!

skabashnyuk commented Sep 24, 2018

Uh oh!

riuvshin commented Sep 24, 2018

Uh oh!

mshaposhnik Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

BarryDrez Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

mshaposhnik Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

mshaposhnik Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

skabashnyuk Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

mshaposhnik Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

mkuznyetsov Sep 24, 2018

Choose a reason for hiding this comment

Uh oh!

skabashnyuk commented Sep 24, 2018

Uh oh!

BarryDrez commented Sep 24, 2018

Uh oh!

mshaposhnik commented Sep 24, 2018

Uh oh!

skabashnyuk commented Sep 24, 2018

Uh oh!

BarryDrez commented Sep 24, 2018

Uh oh!

BarryDrez commented Sep 24, 2018

Uh oh!

mshaposhnik commented Sep 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BarryDrez commented Sep 24, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

mshaposhnik commented Sep 24, 2018 •

edited

Loading