adds OrientDB 3.0.x module #1414

robfrank · 2019-04-19T12:53:37Z

This adds:

OrientDBContainer
tests with both "try with resource" and "Classrule"
an additional test for Gremlin setup
documentation

bumps orientdb version to 3.0.17

kiview · 2019-04-24T19:31:40Z

Hey @robfrank,
thanks a lot for providing this PR for OrientDB.

Since there isn't that much actual OrientDB implementation code inside the module, I wonder if this makes really sense as its own module. WDYT about having this as an example + docs in ./examples instead?

robfrank · 2019-05-23T07:36:38Z

Sorry for my long silence.

I developed the module for two main reasons:

I need it in https://github.com/arcadeAnalytics/arcade-connectors/
there's a Neo4j one, why not an OrientDB?

BTW, I understand that having a great number of modules isn't simply manageable, so I can move the code inside our repo under the com.arcadeanalytics group id and improve the examples/doc on testocontainers to show how to use OrientDB with GenericContainer (that is the way we use it right now)

Or, just merge this :)

WDYT?

rnorth · 2019-05-27T20:51:37Z

I think this probably passes the threshold of adding sufficient value to merit a module of its own. It would be a rather long-winded example snippet.

The remaining sometimes-troublesome area is the reliability of the docker image, drivers and avoidance of flakiness. Hopefully this module is pretty reliable on that front (I'll talk to @bsideup and @kiview about managing this).

Let's review on the basis that it can be accepted as a module, and see where we are.

robfrank · 2019-05-27T21:47:47Z

Hi,
OrientDB is part of SAP and the testcontainer module relies on their Docker official images.

OrientDB is released frequently, at least once a month in this 2019: https://github.com/orientechnologies/orientdb/releases

I can bind the module to "latest" tag, if you think it could be better.

rnorth · 2019-05-28T08:29:46Z

FWIW I've raised this ticket around the current gap in our docs for new module contributors. I'd like to have a bit of a think about the third point specifically before merging this, but otherwise I'm fine with this new module at an overall level: #1503

modules/orientdb/src/test/java/org/testcontainers/containers/OrientDBGremlinTest.java

...orientdb/src/test/java/org/testcontainers/containers/OrientDBContainerWithClassRuleTest.java

removes class rule test add a gremlin test more generic updates to orientdb:3.0.22-tp3

…to orientdb

rnorth · 2019-11-01T09:34:54Z

modules/orientdb/src/test/java/org/testcontainers/containers/OrientDBContainerTest.java

+    @Test
+    public void testContainerLifecycle() {
+        OrientDBContainer container = new OrientDBContainer();
+
+        container.start();
+
+        assertThat(container.isRunning()).isTrue();
+
+        container.stop();
+
+        assertThat(container.isRunning()).isFalse();
+    }


I think this test is unnecessary, since it's a feature of Testcontainers core. Can we remove?

rnorth · 2019-11-01T09:43:10Z

modules/orientdb/src/main/java/org/testcontainers/containers/OrientDBContainer.java

+    public OrientDBContainer withUsername(final String username) {
+        this.username = username;
+        return self();
+    }
+
+    public OrientDBContainer withPassword(final String password) {
+        this.password = password;
+        return self();
+    }


This username and password don't seem to be used to configure the container. In the JDBC database classes, username/password are passed through to the container via environment variables, to tell the container's bootstrap script to create that account.

It doesn't look like OrientDB supports this natively. Line 147 might be an opportunity to create the database and give access to the provided username, but as far as I can tell it's just expecting those credentials to work on line 150.

Is that accurate?

If these methods don't ensure that the account exists, I'd prefer to remove them to avoid confusion, TBH.

You're right. I misunderstood the purpose of the member variables of the Container class, mixing server and client variables. I moved to the getSession method.

modules/orientdb/src/main/java/org/testcontainers/containers/OrientDBContainer.java

rnorth · 2019-11-01T09:59:09Z

modules/orientdb/src/main/java/org/testcontainers/containers/OrientDBContainer.java

+    private static final String DEFAULT_IMAGE_NAME = "orientdb";
+
+    private static final String DEFAULT_TAG = "3.0.24-tp3";
+
+    private static final String DEFAULT_USERNAME = "admin";
+
+    private static final String DEFAULT_PASSWORD = "admin";
+
+    private static final String DEFAULT_DATABASE_NAME = "testcontainers";
+
+    private static final int DEFAULT_BINARY_PORT = 2424;
+
+    private static final int DEFAULT_HTTP_PORT = 2480;
+
+    private static final Logger LOGGER = LoggerFactory.getLogger(OrientDBContainer.class);
+
+    private static final String DOCKER_IMAGE_NAME = DEFAULT_IMAGE_NAME + ":" + DEFAULT_TAG;
+
+    private String databaseName;
+
+    private String username;
+
+    private String password;
+
+    private OrientDB orientDB;
+
+    private ODatabaseSession session;
+
+    private Optional<String> scriptPath = Optional.empty();


Just a tiny style point, but I'd be inclined to reduce the spacing and reorder some of these fields (to group similar fields together, and to reduce visible space taken).

docs/modules/databases/orientdb.md

rnorth

Hi @robfrank, sorry for the epic amount of time it's taken for me to have a proper look at this. I've added some comments.

If you're still interested, and able to, continue this then it'd be great if you could. If you can't, please let us know all the same.

rnorth

@robfrank - thank you for the updates following review, and again I'm sorry that this has been open for so long. Will merge - thanks for the contribution!

Co-authored-by: Richard North <rich.north@gmail.com> Co-authored-by: Sergei Egorov <bsideup@gmail.com>

rnorth · 2020-03-05T16:25:53Z

Released in 1.13.0! Thanks for the contribution @robfrank 😃

Co-authored-by: Richard North <rich.north@gmail.com> Co-authored-by: Sergei Egorov <bsideup@gmail.com>

robfrank added 8 commits February 16, 2019 20:28

initial commit for OrientDB container support module

e3798c4

log configuration for test added

1aa3606

code cleanup

806de50

removes SELF as per code review

fab67ec

adds orientdb module documentation

01208f7

bumps orientdb version to 3.0.17

Merge remote-tracking branch 'upstream/master'

5194b0d

Merge remote-tracking branch 'upstream/master'

8dcb195

adds OrientDB 3.0.x module

cceabb8

robfrank requested review from bsideup, kiview and rnorth as code owners April 19, 2019 12:53

Merge branch 'master' into orientdb

6d061ca

Merge remote-tracking branch 'upstream/master' into orientdb

fbdf916

robfrank and others added 3 commits June 6, 2019 09:41

Merge remote-tracking branch 'upstream/master'

394eb26

Merge remote-tracking branch 'upstream/master' into orientdb

7ab3988

remove unneeded test in OrientDBContainerTest

667b666

bsideup added the type/new module label Jul 19, 2019

bsideup reviewed Jul 19, 2019

View reviewed changes

modules/orientdb/src/test/java/org/testcontainers/containers/OrientDBGremlinTest.java Outdated Show resolved Hide resolved

bsideup reviewed Jul 19, 2019

View reviewed changes

...orientdb/src/test/java/org/testcontainers/containers/OrientDBContainerWithClassRuleTest.java Outdated Show resolved Hide resolved

robfrank and others added 6 commits July 22, 2019 22:28

Merge remote-tracking branch 'upstream/master'

4c4fc2a

removes gremlin tests that rely on custom image

6496d09

removes class rule test add a gremlin test more generic updates to orientdb:3.0.22-tp3

Merge branch 'orientdb' of github.com:robfrank/testcontainers-java in…

2d68734

…to orientdb

merged with master

321804e

Merge branch 'master' into orientdb

4aacece

Merge remote-tracking branch 'upstream/master' into orientdb

82a3bd3

robfrank added 2 commits October 30, 2019 16:31

update to orientdb 3.0.24

7c8bcc3

update to orientdb 3.0.24

6460453

rnorth reviewed Nov 1, 2019

View reviewed changes

modules/orientdb/src/main/java/org/testcontainers/containers/OrientDBContainer.java Show resolved Hide resolved

rnorth reviewed Nov 1, 2019

View reviewed changes

docs/modules/databases/orientdb.md Show resolved Hide resolved

rnorth requested changes Nov 1, 2019

View reviewed changes

rnorth self-assigned this Nov 1, 2019

cleanup following comments on PR

e68cd4e

rnorth approved these changes Feb 5, 2020

View reviewed changes

Merge branch 'master' into orientdb

47d911a

rnorth merged commit 7cb8ef5 into testcontainers:master Feb 5, 2020

artjomka pushed a commit to artjomka/testcontainers-java that referenced this pull request Feb 28, 2020

adds OrientDB 3.0.x module (testcontainers#1414)

d360f44

Co-authored-by: Richard North <rich.north@gmail.com> Co-authored-by: Sergei Egorov <bsideup@gmail.com>

rnorth added this to the 1.13.0 milestone Mar 5, 2020

quincy referenced this pull request in quincy/testcontainers-java May 28, 2020

adds OrientDB 3.0.x module (#1414)

857fd22

Co-authored-by: Richard North <rich.north@gmail.com> Co-authored-by: Sergei Egorov <bsideup@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adds OrientDB 3.0.x module #1414

adds OrientDB 3.0.x module #1414

robfrank commented Apr 19, 2019

kiview commented Apr 24, 2019

robfrank commented May 23, 2019

rnorth commented May 27, 2019

robfrank commented May 27, 2019

rnorth commented May 28, 2019

rnorth Nov 1, 2019

robfrank Nov 2, 2019

rnorth Nov 1, 2019

robfrank Nov 2, 2019

rnorth Nov 1, 2019

robfrank Nov 3, 2019

rnorth left a comment

rnorth left a comment

rnorth commented Mar 5, 2020

adds OrientDB 3.0.x module #1414

adds OrientDB 3.0.x module #1414

Conversation

robfrank commented Apr 19, 2019

kiview commented Apr 24, 2019

robfrank commented May 23, 2019

rnorth commented May 27, 2019

robfrank commented May 27, 2019

rnorth commented May 28, 2019

rnorth Nov 1, 2019

Choose a reason for hiding this comment

robfrank Nov 2, 2019

Choose a reason for hiding this comment

rnorth Nov 1, 2019

Choose a reason for hiding this comment

robfrank Nov 2, 2019

Choose a reason for hiding this comment

rnorth Nov 1, 2019

Choose a reason for hiding this comment

robfrank Nov 3, 2019

Choose a reason for hiding this comment

rnorth left a comment

Choose a reason for hiding this comment

rnorth left a comment

Choose a reason for hiding this comment

rnorth commented Mar 5, 2020