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.

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

Search Oracle image name in classpath #3439

Merged
merged 1 commit into from
Nov 21, 2020

Conversation

eyalkoren
Copy link
Contributor

Not sure why the new code looks for the Oracle image name only in the user home directory and ignores the config file placed in the classpath.
Is that intentional?

Not sure why the new code looks for the Oracle image name only in the user home directory and not in the config file placed in the classpath. 
This brakes my test.
@@ -144,7 +144,7 @@ public String getKafkaImage() {

@Deprecated
public String getOracleImage() {
return getEnvVarOrUserProperty("oracle.container.image", null);
return getEnvVarOrProperty("oracle.container.image", null);
Copy link
Member

Choose a reason for hiding this comment

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

@rnorth shouldn't it be getImage?

Copy link
Member

Choose a reason for hiding this comment

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

Not for this one... getImage provides a substitute name (or the same name) for a given image.

For Oracle we have no default image name at all, so getImage has nothing to substitute. I don't like this, but it's one of those things with the Oracle module.

As a longer term way to tidy this up, I hope that we can work towards removing 'zero-args' instantiation of Oracle and all other containers: so that both for the constructor in code, and in JDBC URLs, an image name must be specified.

@bsideup
Copy link
Member

bsideup commented Nov 8, 2020

@eyalkoren thanks for noticing it! It looks like a leftover from the configuration migration 😅

@AnEmortalKid
Copy link

AnEmortalKid commented Nov 19, 2020

If you're coming here looking for a work around in the interim, I found that using the environment variable works just fine.

<plugin>
  <groupId>org.apache.maven.plugins</groupId>
  <artifactId>maven-surefire-plugin</artifactId>
   <configuration>
     <forkMode>always</forkMode>
     <environmentVariables>
       <!-- interim workaround for https://github.com/testcontainers/testcontainers-java/pull/3439 -->
       <TESTCONTAINERS_ORACLE_CONTAINER_IMAGE>docker/your-non-dcmad-xe-11-image</TESTCONTAINERS_ORACLE_CONTAINER_IMAGE>
       </environmentVariables>
   </configuration>
</plugin>

@rnorth rnorth merged commit dc9e9c1 into testcontainers:master Nov 21, 2020
@eyalkoren eyalkoren deleted the patch-1 branch November 22, 2020 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants