FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add Oracle XE database container support by swissarmykirpan · Pull Request #3 · testcontainers/testcontainers-java · GitHub

Add Oracle XE database container support - #3

Closed
swissarmykirpan wants to merge 3 commits into
testcontainers:masterfrom
swissarmykirpan:OracleXE
Closed

Add Oracle XE database container support#3
swissarmykirpan wants to merge 3 commits into
testcontainers:masterfrom
swissarmykirpan:OracleXE

Conversation

Copy link
Copy Markdown
Member

Created an Oracle Container and simple test

rnorth commented Apr 28, 2015

Copy link
Copy Markdown
Member

Thanks Gurpreet! Great work!

Technically it looks good to me, though I've not been able to run it yet (OJDBC dependency issues..)

I'd just like to think through a couple of issues associated with the difficulty of obtaining proprietary Oracle assets for testing and actual use:

  • The OJDBC JAR can't be bundled, and it looks like Oracle's new maven repository needs registration. We can probably sort this out fairly easily for testing though (and we don't have a compile time/direct dependency on any drivers, deliberately)
  • I want to understand the licence situation of the docker Oracle XE image. I've been thinking about making it possible to specify a full docker image name in the JDBC URL and/or custom docker registries, and these might be necessary to allow teams to use their own privately managed images.

Let's talk about these some time.

Copy link
Copy Markdown
Member Author

I've pulled in your override commit into my branch and made the changes to support Oracle.
In regards to the docker Oracle XE Image - the dockerfile is on github so fine to use. I've had a look at http://docs.oracle.com/cd/B25329_01/doc/license.102/b25456/toc.htm and found it to be ok (would be worth you having a look).

rnorth commented Jun 22, 2015

Copy link
Copy Markdown
Member

Hi Gurpreet
0.9.3 is released now with the modularization changes we discussed - 0.9.4 will be the next version, and is ready to receive your change.

Please could you refactor your PR into the altered project structure?

Please:

  • make a new module under /modules/oracle-xe, but this should not be included in the default build
  • set up a maven profile 'proprietary-deps' that includes the oracle-xe module into the build
  • add your details to the root POM in the developer section (we should probably have an AUTHORS file too)...

Copy link
Copy Markdown
Member Author

Hi Rich. I've updated the code to reflect the changes requested in your last comment.

Comment thread modules/oracle-xe/pom.xml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I imagine the Oracle JDBC driver needs to be added here - please make sure you're using the standard groupId:artifactId for it (can't remember what the convention is off the top of my head)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Added

Comment thread modules/oracle-xe/pom.xml Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Scope here and for dbutils seems to have changed - should be test, I think!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ah - sorry I just noticed: please use plain JDBC for the container liveness check, not Hikari.

Much as I love Hikari and think everyone should use it for their DB connection pooling needs, I'm not quite yet ready to force it upon everybody as a compile-scoped dependency. We should be able to use regular JDBC to do the liveness check.

(NB I know the presence of the zexec library goes against the above philosophy, and I'm mulling replacing it with a JDK-standard approach!)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Keep the feedback coming!

…leDataSource

Changed Hikari Maven Dependency scope to Test
No longer exposing Port 22
rnorth added this to the 0.9.5 milestone Jun 26, 2015
rnorth self-assigned this Jun 26, 2015
rnorth changed the title Add Oracle Support (WIP) Add Oracle XE database container support Jun 27, 2015

rnorth commented Jun 27, 2015

Copy link
Copy Markdown
Member

Hmm, attempting to merge this but the Oracle XE container seems to be blowing up. I can't understand how I'd be the first to see this, but it seems that the container has the wrong permissions set on its /var/run/sshd folder:

❯ docker run -p 1521:1521 alexeiled/docker-oracle-xe-11g                                                                                                                                                                        
Starting Oracle Net Listener.
Starting Oracle Database 11g Express Edition instance.

/var/run/sshd must be owned by root and not group or world-writable.

~/projects/tc git$master  28.9s
❯

I've confirmed that this does seem to be a trait of the container:

❯ docker run -p 1521:1521 -it alexeiled/docker-oracle-xe-11g /bin/bash                                                                                                                                                       
root@a386c8710f43:/# ls -al /var/run/sshd
total 8
drwxr-xr-x 2 oracle dba  4096 Jan 11  2014 .
drwxr-xr-x 7 root   root 4096 Jan 12  2014 ..
root@a386c8710f43:/# exit

I'm going to see whether the wnameless fork fares any better; it seems to be much more actively maintained at any rate...

rnorth closed this in 0268bfd Jun 27, 2015

rnorth commented Jun 27, 2015

Copy link
Copy Markdown
Member

Went with wnameless in the end - @gusohal please confirm whether this is an issue for you.

Copy link
Copy Markdown
Member Author

No problem at all. I didn't seem to have any issues with the original container. Weird.

bsideup pushed a commit that referenced this pull request Jul 25, 2018
shsr04 referenced this pull request in qaware/testcontainers-java Oct 15, 2021
…ficates

Add configuration setting that allows disabling TLS certificate validation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants


Back | FazBrowse Home | New Git URL