Skip to content

[SPARK-29567][TESTS] Update JDBC Integration Test Docker Images #26224

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

Closed
wants to merge 1 commit into from
Closed

[SPARK-29567][TESTS] Update JDBC Integration Test Docker Images #26224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion ...er-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MySQLIntegrationSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import org.apache.spark.tags.DockerTest
@DockerTest
class MySQLIntegrationSuite extends DockerJDBCIntegrationSuite {
override val db = new DatabaseOnDocker {
override val imageName = "mysql:5.7.9"
override val imageName = "mysql:5.7.28"
override val env = Map(
"MYSQL_ROOT_PASSWORD" -> "rootpass"
)
Expand All @@ -39,6 +39,8 @@ class MySQLIntegrationSuite extends DockerJDBCIntegrationSuite {
}

override def dataPreparation(conn: Connection): Unit = {
// Since MySQL 5.7.14+, we need to disable strict mode
Copy link
Member

@maropu maropu Oct 23, 2019

Choose a reason for hiding this comment

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

just out of curiosity; why we need to disable the mode in this mysql?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Oct 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

As I mentioned in the PR description, MySQL baned 0000-00-00 00:00:00. There is a SQL mode for that. I enumerated and tested all versions. It starts to ban on 5.7.14.

Copy link
Member

@maropu maropu Oct 23, 2019

Choose a reason for hiding this comment

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

oh, I missed that. thanks.

conn.prepareStatement("SET GLOBAL sql_mode = ''").executeUpdate()
conn.prepareStatement("CREATE DATABASE foo").executeUpdate()
conn.prepareStatement("CREATE TABLE tbl (x INTEGER, y TEXT(8))").executeUpdate()
conn.prepareStatement("INSERT INTO tbl VALUES (42,'fred')").executeUpdate()
Expand Down
2 changes: 1 addition & 1 deletion ...integration-tests/src/test/scala/org/apache/spark/sql/jdbc/PostgresIntegrationSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import org.apache.spark.tags.DockerTest
@DockerTest
class PostgresIntegrationSuite extends DockerJDBCIntegrationSuite {
override val db = new DatabaseOnDocker {
override val imageName = "postgres:11.4"
override val imageName = "postgres:12.0-alpine"
Copy link
Member

@maropu maropu Oct 23, 2019

Choose a reason for hiding this comment

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

12.0-alpine instead of 12.0?

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Oct 23, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Yes. This is another improvement. It reduces the image size a lot, 312MB -> 72.8MB. This is good for CI/CI testing environment.

$ docker images | grep postgres
postgres                         12.0-alpine          5b681acb1cfc        2 days ago          72.8MB
postgres                         11.4                 53912975086f        3 months ago        312MB

Copy link
Member Author

@dongjoon-hyun dongjoon-hyun Oct 23, 2019

Choose a reason for hiding this comment

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

I'll add this to the PR description.

Copy link
Member

@maropu maropu Oct 23, 2019

Choose a reason for hiding this comment

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

Thanks! Yea, it looks nice.

override val env = Map(
"POSTGRES_PASSWORD" -> "rootpass"
)
Expand Down