[SPARK-27155][TEST] Parameterize Oracle docker image name #24086

lipzhu · 2019-03-14T03:03:00Z

What changes were proposed in this pull request?

Update Oracle docker image name.

How was this patch tested?

./build/mvn test -Pdocker-integration-tests -pl :spark-docker-integration-tests_2.12

wangyum · 2019-03-14T03:10:17Z

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

@@ -55,7 +56,7 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSQLCo
  import testImplicits._

  override val db = new DatabaseOnDocker {
-    override val imageName = "wnameless/oracle-xe-11g:16.04"
+    override val imageName = "deepdiver/docker-oracle-xe-11g:2.0"


Why do we need update this image?

hmm.. who owns/licenses these images...

Why do we need update this image?

I see, but is the new image 'legal' either? is there an official one? and if not, should we be using or testing against it?

I also have the same concerns on the legal stuff. If we don't have an official docker image, we had better remove OracleIntegrationSuite from our docker test.

Seems there is no official Oracle docker image, can we Ignore this Test to pass the docker integration test?

I think it should be deleted if there is no legitimate way to test this

agreed there.. one possible approach is to leave the image name as a parameter and document that someone needs to build one..

Looks like this is how you build it:
https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md
but looks like indeed you have to supply the binaries as they're not freely available.
Yes at best I think we'd need to disable this test and parameterize it, as it's not going to work reliably anyway.

lipzhu · 2019-03-15T11:04:53Z

Add a parameter SPARK_ORACLE_DOCKER_IMAGE_NAME for Oracle tests.

srowen

Is there a way to disable this test also, if it's run automatically? or at least skip its execution if no image is specified?

srowen · 2019-03-15T13:47:56Z

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

@@ -55,7 +55,8 @@ class OracleIntegrationSuite extends DockerJDBCIntegrationSuite with SharedSQLCo
  import testImplicits._

  override val db = new DatabaseOnDocker {
-    override val imageName = "wnameless/oracle-xe-11g:16.04"
+    override val imageName = sys.env.getOrElse("SPARK_ORACLE_DOCKER_IMAGE_NAME",
+      "wnameless/oracle-xe-11g:16.04")


We can't even have a default; this default doesn't work

srowen · 2019-03-18T00:22:26Z

OK, is the point that this will never be run anyway unless someone runs it manually? Then I think this is OK if you remove the default (that doesn't exist anyway) and add a few more comments pointing to the oracle repo about how the image can be built.

dilipbiswal · 2019-03-20T14:18:58Z

Should we update the PR description to reflect what its doing now in case some one refers to it later ?

SparkQA · 2019-03-20T16:07:36Z

Test build #4646 has finished for PR 24086 at commit 32c1144.

This patch fails Scala style tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2019-03-20T16:14:25Z

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

- * 2. Start docker - sudo service docker start
- * 3. Download oracle 11g driver jar and put it in maven local repo:
+ * 1. Build Oracle database in Docker, please refer below link about how to.
+ *    @see https://github.com/oracle/docker-images/blob/master/OracleDatabase/SingleInstance/README.md


The line is too long, and the @see tag doesn't work here I think. Put "See" on the previous line and unindent to see if that gets it to under 100 characters

lipzhu · 2019-03-21T08:35:44Z

reset this please.

SparkQA · 2019-03-21T11:36:09Z

Test build #4651 has started for PR 24086 at commit 21369b9.

dongjoon-hyun · 2019-03-21T16:36:53Z

...cker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/OracleIntegrationSuite.scala

+      """
+        |INSERT INTO ts_with_timezone VALUES (
+        |1, to_timestamp_tz('1999-12-01 11:00:00 UTC','YYYY-MM-DD HH:MI:SS TZR'))
+         """.stripMargin).executeUpdate()


indentation?

Or @lipzhu why did this part change? I may be missing it but it seems like just a formatting change. I don't think it's necessary

@srowen Yes, it is just a formatting change, I see these 2 codes' style is not consistent with the others in this suite. If you think it it not necessary, I will change them back.

SparkQA · 2019-03-25T07:05:02Z

Test build #4660 has finished for PR 24086 at commit e7bc15d.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2019-03-25T19:15:53Z

Test build #4661 has finished for PR 24086 at commit e7bc15d.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

srowen · 2019-03-25T20:17:52Z

Merged to master

dilipbiswal · 2019-05-02T18:11:16Z

@dongjoon-hyun Hi, i was trying to follow the instructions in OracleIntegrationSuite to try and build an image to run the oracle tests. I was trying it on mac.. Haven't been able to. Were you able to run this successfully ? Please let me know .. I get the following error when i try to build the image.

Building image 'oracle/database:11.2.0.2-xe' ...
Sending build context to Docker daemon  2.351GB
Step 1/10 : FROM oraclelinux:7-slim
 ---> f7512ac13c1b
Step 2/10 : MAINTAINER Gerald Venzl <gerald.venzl@oracle.com>
 ---> Running in 626a011e37a1
Removing intermediate container 626a011e37a1
 ---> 3a1f198c974f
Step 3/10 : ENV ORACLE_BASE=/u01/app/oracle     ORACLE_HOME=/u01/app/oracle/product/11.2.0/xe     ORACLE_SID=XE     INSTALL_FILE_1="oracle-xe-11.2.0-1.0.x86_64.rpm.zip"     INSTALL_DIR="$HOME/install"     CONFIG_RSP="xe.rsp"     RUN_FILE="runOracle.sh"     PWD_FILE="setPassword.sh"     CHECK_DB_FILE="checkDBStatus.sh"
 ---> Running in 969673e55df7
Removing intermediate container 969673e55df7
 ---> 441b63aabd8a
Step 4/10 : ENV PATH=$ORACLE_HOME/bin:$PATH
 ---> Running in 52469448bf1b
Removing intermediate container 52469448bf1b
 ---> b80dbfddd0a9
Step 5/10 : COPY $INSTALL_FILE_1 $CONFIG_RSP $RUN_FILE $PWD_FILE $CHECK_DB_FILE $INSTALL_DIR/
COPY failed: stat /var/lib/docker/tmp/docker-builder372031542/oracle-xe-11.2.0-1.0.x86_64.rpm.zip: no such file or directory

dongjoon-hyun · 2019-05-02T18:25:51Z

Hi, @dilipbiswal .
This PR doesn't imply that oracle/database:11.2.0.2-xe works, does this?

srowen · 2019-05-02T18:31:12Z

Yeah, I'm not clear one would be able to use this at all without an Oracle license. I guess I'd be surprised if there's any free public Oracle DB image out there.

dilipbiswal · 2019-05-02T18:32:33Z

@dongjoon-hyun Oh... actually i just needed a little help to get the oracle tests to run as i am trying to fix a defect that would require me to add a test in this suite. I just used this recent PR to ask the question :-)

dilipbiswal · 2019-05-02T19:46:27Z

@dongjoon-hyun A quick update: I got hold of an un-official image and got the tests to pass. So i am not blocked at the moment :-)

update oracle docker image name

62f9fd3

lipzhu changed the title ~~update oracle docker image name~~ [SPARK-27155][Build]update oracle docker image name Mar 14, 2019

wangyum reviewed Mar 14, 2019

View reviewed changes

add parameter SPARK_ORACLE_DOCKER_IMAGE_NAME for oracle integration test

1b1da25

srowen requested changes Mar 15, 2019

View reviewed changes

remove wnameless/oracle-xe-11g:16.04 image as it doesn't exist

32c1144

lipzhu mentioned this pull request Mar 19, 2019

[SPARK-27168][SQL][TEST] Add docker integration test for MsSql server #24099

Closed

srowen approved these changes Mar 20, 2019

View reviewed changes

srowen reviewed Mar 20, 2019

View reviewed changes

beauty code

21369b9

dongjoon-hyun reviewed Mar 21, 2019

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-27155][Build]update oracle docker image name~~ [SPARK-27155][TEST] Parameterize Oracle docker image name Mar 21, 2019

remove unncessary code change for SPARK-27155

e7bc15d

srowen approved these changes Mar 25, 2019

View reviewed changes

dongjoon-hyun approved these changes Mar 25, 2019

View reviewed changes

srowen closed this in 1f2564d Mar 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-27155][TEST] Parameterize Oracle docker image name #24086

[SPARK-27155][TEST] Parameterize Oracle docker image name #24086

lipzhu commented Mar 14, 2019

wangyum Mar 14, 2019

felixcheung Mar 14, 2019

lipzhu Mar 14, 2019

srowen Mar 14, 2019

dongjoon-hyun Mar 14, 2019 •

edited

lipzhu Mar 15, 2019

srowen Mar 15, 2019

felixcheung Mar 15, 2019 •

edited

srowen Mar 15, 2019

lipzhu commented Mar 15, 2019

srowen left a comment

srowen Mar 15, 2019

srowen commented Mar 18, 2019

dilipbiswal commented Mar 20, 2019

SparkQA commented Mar 20, 2019

srowen Mar 20, 2019

lipzhu commented Mar 21, 2019

SparkQA commented Mar 21, 2019

dongjoon-hyun Mar 21, 2019

srowen Mar 22, 2019

lipzhu Mar 25, 2019

SparkQA commented Mar 25, 2019

SparkQA commented Mar 25, 2019

srowen commented Mar 25, 2019

dilipbiswal commented May 2, 2019

dongjoon-hyun commented May 2, 2019

srowen commented May 2, 2019

dilipbiswal commented May 2, 2019

dilipbiswal commented May 2, 2019

[SPARK-27155][TEST] Parameterize Oracle docker image name #24086

[SPARK-27155][TEST] Parameterize Oracle docker image name #24086

Conversation

lipzhu commented Mar 14, 2019

What changes were proposed in this pull request?

How was this patch tested?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun Mar 14, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixcheung Mar 15, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lipzhu commented Mar 15, 2019

srowen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

srowen commented Mar 18, 2019

dilipbiswal commented Mar 20, 2019

SparkQA commented Mar 20, 2019

Choose a reason for hiding this comment

lipzhu commented Mar 21, 2019

SparkQA commented Mar 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Mar 25, 2019

SparkQA commented Mar 25, 2019

srowen commented Mar 25, 2019

dilipbiswal commented May 2, 2019

dongjoon-hyun commented May 2, 2019

srowen commented May 2, 2019

dilipbiswal commented May 2, 2019

dilipbiswal commented May 2, 2019

dongjoon-hyun Mar 14, 2019 •

edited

felixcheung Mar 15, 2019 •

edited