Add InfluxDB v2 support #3669

raminqaf · 2021-01-07T15:35:43Z

This PR fixes #3670

Every item on this list will require judgement by the Testcontainers core maintainers. Exceptions will sometimes be possible; items with should are more likely to be negotiable than those items with must.

Default docker image

Should be a Docker Hub official image, or published by a reputable source (ideally the company or organisation that officially supports the technology)
Should have a verifiable open source Dockerfile and a way to view the history of changes
MUST show general good practices regarding container image tagging - e.g. we do not use latest tags, and we do not use tags that may be mutated in the future
MUST be legal for Testcontainers developers and Testcontainers users to pull and use. Mechanisms exist to allow EULA acceptance to be signalled, but images that can be used without a licence are greatly preferred.

Module dependencies

The module should use as few dependencies as possible,
Regarding libraries, either:
- they should be compileOnly if they are likely to already be on the classpath for users' tests (e.g. client libraries or drivers)
- they can be implementation (and thus transitive dependencies) if they are very unlikely to conflict with users' dependencies.
If client libraries are used to test or use the module, these MUST be legal for Testcontainers developers and Testcontainers users to download and use.

API (e.g. `MyModuleContainer` class)

Favour doing the right thing, and least surprising thing, by default
Ensure that method and parameter names are easy to understand. Many users will ignore documentation, so IDE-based substitutes (autocompletion and Javadocs) should be intuitive.
The module's public API should only handle standard JDK data types and MUST not expose data types that come from compileOnly dependencies. This is to reduce the risk of compatibility problems with future versions of third party libraries.

Documentation

Every module MUST have a dedicated documentation page containing:
- A high level overview
- A usage example
- If appropriate, basic API documentation or further usage guidelines
- Dependency information
- Acknowledgements, if appropriate
Consider that many users will not read the documentation pages - even if the first person to add it to a project does, people reading/updating the code in the future may not. Try and avoid the need for critical knowledge that is only present in documentation.

rnorth · 2021-01-10T12:29:57Z

Thanks for the contribution @raminqaf

I think one thing we need to be really careful about is the breaking-change nature of this. Is there no way at all to make the InfluxDBContainer be compatible with both v1.x and v2.x of InfluxDB?

We need to avoid a situation where an upgrade of Testcontainers prevents people from being able to run their existing tests that rely on InfluxDB v1.x.

raminqaf · 2021-01-10T12:37:44Z

@rnorth unfortunately not. Another way to add support for InfluxDB 2.x and keep the 1.x supported is to add a new and sperated module like influxDB2.

rnorth · 2021-01-10T12:44:55Z

What if we kept it in the same module, but have a InfluxDBContainerV2 class? We could also rename the current class to InfluxDBContainerV1 and have a @Deprecated class InfluxDBContainer extends InfluxDBContainerV1 to maintain binary compatibility.

raminqaf · 2021-01-10T12:46:24Z

@rnorth Sounds good to me! I will add your suggestions and push them to the PR.

raminqaf · 2021-03-11T08:42:14Z

@rnorth any updates on this PR?

vcvitaly · 2021-03-11T12:35:10Z

I had a quick read and everything is ok at the first glance 👀

raminqaf · 2021-03-14T17:14:21Z

@vektory79 Thanks!
Would be could if we could merge this. Since we are using the code in our project. I would like to have it from testcontainers dependency, rather than having it in our project directly 😉 @rnorth @kiview @bsideup

gjemp · 2021-03-23T10:04:20Z

hi,
some updates should be done

influxdbV2ClientVersion = '1.14.0'

there is 2.0.0 out.

the example about v2 is a bit missleading i think cause the default for pure v2 s now token based . The user and password is only for the v1=>v2 upgraded instances.

raminqaf · 2021-03-28T15:32:45Z

hi,
some updates should be done

influxdbV2ClientVersion = '1.14.0'

there is 2.0.0 out.

the example about v2 is a bit missleading i think cause the default for pure v2 s now token based . The user and password is only for the v1=>v2 upgraded instances.

Thanks for the comment and your feedback on the PR. By the time I created this PR the version was 1.14.0 but I will update it to the newest one. The authentication and setup of the InfluxDB could be done either through username and password or through the token. As you can see on the InfluxDB Docker setup page. And according to the setup flags documentation the username and password are still valid arguments to do the authentication. Besides you can use these credentials to log in, during debug mode, to the dashboard and query the data.

…qaf/testcontainers-java into feature/influxdb-v2-test-container

gjemp · 2021-03-29T06:38:42Z

Thanks for the comment and your feedback on the PR. By the time I created this PR the version was 1.14.0 but I will update it to the newest one. The authentication and setup of the InfluxDB could be done either through username and password or through the token. As you can see on the InfluxDB Docker setup page. And according to the setup flags documentation the username and password are still valid arguments to do the authentication. Besides you can use these credentials to log in, during debug mode, to the dashboard and query the data.

Thanks for update. Any idea when we get the review done ? Would really like to have the update as release.

raminqaf · 2021-03-29T06:42:56Z

Thanks for the comment and your feedback on the PR. By the time I created this PR the version was 1.14.0 but I will update it to the newest one. The authentication and setup of the InfluxDB could be done either through username and password or through the token. As you can see on the InfluxDB Docker setup page. And according to the setup flags documentation the username and password are still valid arguments to do the authentication. Besides you can use these credentials to log in, during debug mode, to the dashboard and query the data.

Thanks for update. Any idea when we get the review done ? Would really like to have the update as release.

Unfortunately not. In order to merge the PR, one of the three reviewers (@rnorth @bsideup @bsideup) should approved the PR first.

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainerV2.java

raminqaf · 2022-09-09T12:57:02Z

@kiview Is this good to merge?

eddumelendez

@raminqaf thanks once again! I just noticed some important changes that we should address before merge.

eddumelendez · 2022-10-10T15:19:10Z

modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerTest.java

+        try (
+            final InfluxDBContainer influxDBContainer = new InfluxDBContainer(InfluxDBTestUtils.INFLUXDB_V2_TEST_IMAGE)
+        ) {
+            influxDBContainer
+                .withUsername(USERNAME)
+                .withPassword(PASSWORD)
+                .withOrganization(ORG)
+                .withBucket(BUCKET)
+                .withRetention(RETENTION);


Suggested change

try (

final InfluxDBContainer influxDBContainer = new InfluxDBContainer(InfluxDBTestUtils.INFLUXDB_V2_TEST_IMAGE)

) {

influxDBContainer

.withUsername(USERNAME)

.withPassword(PASSWORD)

.withOrganization(ORG)

.withBucket(BUCKET)

.withRetention(RETENTION);

try (

final InfluxDBContainer influxDBContainer = new InfluxDBContainer(InfluxDBTestUtils.INFLUXDB_V2_TEST_IMAGE)

.withUsername(USERNAME)

.withPassword(PASSWORD)

.withOrganization(ORG)

.withBucket(BUCKET)

.withRetention(RETENTION);

) {

eddumelendez · 2022-10-10T15:21:26Z

docs/modules/databases/influxdb.md

+In the following example you will find a snippet to create an InfluxDB client using the official Java client:
+
+<!--codeinclude-->
+[InfluxDBTestUtils.getInfluxDBClient()](../../../modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBTestUtils.java)


same here about use the code that is required

eddumelendez · 2022-10-10T15:21:46Z

docs/modules/databases/influxdb.md

+Running a `InfluxDBContainer` as a stand-in for InfluxDB in a test:
+
+<!--codeinclude-->
+[InfluxDBContainerV1Test](../../../modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerV1Test.java)


same here about use the code that is required

eddumelendez · 2022-10-10T15:21:50Z

docs/modules/databases/influxdb.md

+In the following example you will find a snippet to create an InfluxDB client using the official Java client:
+
+<!--codeinclude-->
+[InfluxDBContainer.getNewInfluxDB()](../../../modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainer.java)


same here about use the code that is required

missing the block here too

eddumelendez · 2022-10-10T15:23:55Z

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainer.java

     */
    public String getUrl() {
-        return "http://" + getHost() + ":" + getLivenessCheckPort();
+        return "http://" + this.getHost() + ":" + this.getMappedPort(INFLUXDB_PORT);


Suggested change

return "http://" + this.getHost() + ":" + this.getMappedPort(INFLUXDB_PORT);

return "http://" + getHost() + ":" + getMappedPort(INFLUXDB_PORT);

eddumelendez · 2022-10-10T15:24:06Z

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainer.java

    public InfluxDB getNewInfluxDB() {
-        InfluxDB influxDB = InfluxDBFactory.connect(getUrl(), username, password);
-        influxDB.setDatabase(database);
+        final InfluxDB influxDB = InfluxDBFactory.connect(this.getUrl(), this.username, this.password);


Suggested change

final InfluxDB influxDB = InfluxDBFactory.connect(this.getUrl(), this.username, this.password);

final InfluxDB influxDB = InfluxDBFactory.connect(getUrl(), this.username, this.password);

eddumelendez · 2022-10-10T15:25:02Z

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainer.java

 */
-public class InfluxDBContainer<SELF extends InfluxDBContainer<SELF>> extends GenericContainer<SELF> {
+public class InfluxDBContainer extends GenericContainer<InfluxDBContainer> {


for compatibility, we need to rollback this change. We can apply it again in version 2.x

eddumelendez · 2022-10-10T15:27:12Z

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainer.java

+        this.isAtLeastMajorVersion2 =
+            new ComparableVersion(dockerImageName.getVersionPart()).isGreaterThanOrEqualTo("2.0.0");


@kiview I wonder if we can remove this check and use the version provided via ENV here. In order to do this we need to make a dockerClient.inspectImageCmd(image) at the configure method. WDYT?

@eddumelendez I think I would prefer your suggested approach. It is still not a super strong indicator for custom images, but I believe more explicit than the tag version.

I will leave it as is for now. I think we need to think about how to address them for custom images.

eddumelendez · 2022-10-10T15:27:58Z

docs/modules/databases/influxdb.md

+Running a `InfluxDBContainer` as a stand-in for InfluxDB in a test:
+
+<!--codeinclude-->
+[InfluxDBContainerTest](../../../modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerTest.java)


Currently, the whole test class is included in the docs. Use blocks in order to use only what is required. See https://github.com/testcontainers/testcontainers-java/blob/main/docs/modules/pulsar.md?plain=1#L11-L13 and https://github.com/testcontainers/testcontainers-java/blob/main/modules/pulsar/src/test/java/org/testcontainers/containers/PulsarContainerTest.java#L32-L34

this one is missing

@eddumelendez Done!

…qaf/testcontainers-java into feature/influxdb-v2-test-container

raminqaf · 2022-10-15T12:55:45Z

@eddumelendez Thanks for the review! I added all of them!

…qaf/testcontainers-java into feature/influxdb-v2-test-container

eddumelendez · 2022-10-19T16:25:40Z

It's failing because of

Error: eckstyle] [ERROR] /home/runner/work/testcontainers-java/testcontainers-java/modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerTest.java:32:5: 'VARIABLE_DEF' should be separated from previous line. [EmptyLineSeparator]
Error: eckstyle] [ERROR] /home/runner/work/testcontainers-java/testcontainers-java/modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerV1Test.java:4:62: Using a static member import should be avoided - org.testcontainers.containers.InfluxDBTestUtils.createInfluxDBWithUrl. [AvoidStaticImport]

You can reproduce it locally by running ./gradlew :influxdb:checkstyleMain :influxdb:checkstyleTest

eddumelendez

thanks a lot for your patience @raminqaf ! I think we are just polishing the docs now and from my side LGTM.

modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerTest.java

raminqaf · 2022-10-20T13:22:36Z

@eddumelendez I added all the reviews and the checkstyle checks!

kiview · 2022-11-14T16:57:41Z

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainer.java

+        this.isAtLeastMajorVersion2 =
+            new ComparableVersion(dockerImageName.getVersionPart()).isGreaterThanOrEqualTo("2.0.0");


@eddumelendez I think I would prefer your suggested approach. It is still not a super strong indicator for custom images, but I believe more explicit than the tag version.

eddumelendez · 2022-11-15T17:46:50Z

thank you so much for your contribution and all work you've done, @raminqaf ! This is now in main branch and will be available in next version.

raminqaf added 2 commits January 7, 2021 13:39

update InfluxDBContainer class for v2+

1d7a20f

update influxdb.md

60d8970

raminqaf requested review from bsideup, kiview and rnorth as code owners January 7, 2021 15:35

raminqaf added 2 commits January 7, 2021 17:15

add retention time and update influxdb.md

3aa16a5

made retentiion units lable package private

898b4a9

raminqaf added 4 commits January 10, 2021 14:35

add influxDB v1.x code

3ea5560

Merge branch 'master' into feature/influxdb-v2-test-container

5ce658b

Merge branch 'master' into feature/influxdb-v2-test-container

dc56931

Merge branch 'master' into feature/influxdb-v2-test-container

d70ed4d

raminqaf mentioned this pull request Mar 14, 2021

[BAHIR-274] Add Flink InfluxDBv2.0 Connector apache/bahir-flink#114

Merged

raminqaf added 2 commits March 28, 2021 18:09

bump inflxudb client version up to 2.0.0

31ecbe3

Merge branch 'feature/influxdb-v2-test-container' of github.com:ramin…

41e366a

…qaf/testcontainers-java into feature/influxdb-v2-test-container

raminqaf added 2 commits April 9, 2021 09:09

Merge branch 'master' into feature/influxdb-v2-test-container

9c3b224

Merge branch 'master' into feature/influxdb-v2-test-container

0be00c9

rnorth reviewed Jun 17, 2021

View reviewed changes

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainerV2.java Outdated Show resolved Hide resolved

rnorth reviewed Jun 17, 2021

View reviewed changes

modules/influxdb/src/main/java/org/testcontainers/containers/InfluxDBContainerV2.java Outdated Show resolved Hide resolved

Andy2003 suggested changes Jun 17, 2021

View reviewed changes

raminqaf requested review from kiview and removed request for kiview August 23, 2022 14:15

raminqaf added 2 commits August 23, 2022 16:16

Merge branch 'master' into feature/influxdb-v2-test-container

f03ac43

Merge branch 'master' into feature/influxdb-v2-test-container

ccb1dac

Merge branch 'main' into feature/influxdb-v2-test-container

4b365b0

eddumelendez requested changes Oct 10, 2022

View reviewed changes

raminqaf and others added 3 commits October 15, 2022 14:54

Add reviews

a1d1245

Merge branch 'feature/influxdb-v2-test-container' of github.com:ramin…

cb0c2c0

…qaf/testcontainers-java into feature/influxdb-v2-test-container

Merge branch 'main' into feature/influxdb-v2-test-container

8ba2714

raminqaf added 3 commits October 18, 2022 09:00

Fix docs

8f6f2a5

Merge branch 'feature/influxdb-v2-test-container' of github.com:ramin…

773328c

…qaf/testcontainers-java into feature/influxdb-v2-test-container

Fix docs

aa1e2ca

eddumelendez requested changes Oct 19, 2022

View reviewed changes

modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerTest.java Outdated Show resolved Hide resolved

modules/influxdb/src/test/java/org/testcontainers/containers/InfluxDBContainerTest.java Outdated Show resolved Hide resolved

raminqaf and others added 3 commits October 20, 2022 15:18

add reviews

1d817fe

add checkstyle

adab811

Merge branch 'main' into feature/influxdb-v2-test-container

8a190d0

eddumelendez dismissed rnorth’s stale review via 8a190d0 October 29, 2022 16:47

eddumelendez previously approved these changes Nov 8, 2022

View reviewed changes

kiview previously approved these changes Nov 14, 2022

View reviewed changes

Polish

627a01c

eddumelendez dismissed stale reviews from kiview and themself via 627a01c November 15, 2022 17:31

eddumelendez added this to the next milestone Nov 15, 2022

eddumelendez merged commit e3ec7d4 into testcontainers:main Nov 15, 2022

raminqaf deleted the feature/influxdb-v2-test-container branch November 28, 2022 12:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add InfluxDB v2 support #3669

Add InfluxDB v2 support #3669

raminqaf commented Jan 7, 2021 •

edited

rnorth commented Jan 10, 2021

raminqaf commented Jan 10, 2021

rnorth commented Jan 10, 2021

raminqaf commented Jan 10, 2021

raminqaf commented Mar 11, 2021

vcvitaly commented Mar 11, 2021

raminqaf commented Mar 14, 2021

gjemp commented Mar 23, 2021 •

edited

raminqaf commented Mar 28, 2021 •

edited

gjemp commented Mar 29, 2021

raminqaf commented Mar 29, 2021

raminqaf commented Sep 9, 2022

eddumelendez left a comment

eddumelendez Oct 10, 2022

eddumelendez Oct 10, 2022

eddumelendez Oct 10, 2022

eddumelendez Oct 10, 2022

eddumelendez Oct 17, 2022

raminqaf Oct 18, 2022

eddumelendez Oct 10, 2022

eddumelendez Oct 10, 2022

eddumelendez Oct 10, 2022

eddumelendez Oct 10, 2022

kiview Nov 14, 2022

eddumelendez Nov 15, 2022

eddumelendez Oct 10, 2022

eddumelendez Oct 17, 2022

raminqaf Oct 18, 2022

raminqaf commented Oct 15, 2022

eddumelendez commented Oct 19, 2022

eddumelendez left a comment

raminqaf commented Oct 20, 2022

kiview Nov 14, 2022

eddumelendez commented Nov 15, 2022

	return "http://" + this.getHost() + ":" + this.getMappedPort(INFLUXDB_PORT);
	return "http://" + getHost() + ":" + getMappedPort(INFLUXDB_PORT);

	final InfluxDB influxDB = InfluxDBFactory.connect(this.getUrl(), this.username, this.password);
	final InfluxDB influxDB = InfluxDBFactory.connect(getUrl(), this.username, this.password);

		this.isAtLeastMajorVersion2 =
		new ComparableVersion(dockerImageName.getVersionPart()).isGreaterThanOrEqualTo("2.0.0");

Add InfluxDB v2 support #3669

Add InfluxDB v2 support #3669

Conversation

raminqaf commented Jan 7, 2021 • edited

Default docker image

Module dependencies

API (e.g. MyModuleContainer class)

Documentation

rnorth commented Jan 10, 2021

raminqaf commented Jan 10, 2021

rnorth commented Jan 10, 2021

raminqaf commented Jan 10, 2021

raminqaf commented Mar 11, 2021

vcvitaly commented Mar 11, 2021

raminqaf commented Mar 14, 2021

gjemp commented Mar 23, 2021 • edited

raminqaf commented Mar 28, 2021 • edited

gjemp commented Mar 29, 2021

raminqaf commented Mar 29, 2021

raminqaf commented Sep 9, 2022

eddumelendez left a comment

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

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

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

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

raminqaf commented Oct 15, 2022

eddumelendez commented Oct 19, 2022

eddumelendez left a comment

Choose a reason for hiding this comment

raminqaf commented Oct 20, 2022

Choose a reason for hiding this comment

eddumelendez commented Nov 15, 2022

raminqaf commented Jan 7, 2021 •

edited

API (e.g. `MyModuleContainer` class)

gjemp commented Mar 23, 2021 •

edited

raminqaf commented Mar 28, 2021 •

edited