Add Consul module by julb · Pull Request #4683 · testcontainers/testcontainers-java

julb · 2021-11-17T14:07:23Z

No description provided.

kiview · 2021-11-17T15:28:17Z

Hi @julb, thanks a lot for contributing this module.

Looking at the logic around runInitCommands(), it seems that this module makes using Consul with Testcontainers considerably easier, so theoretically we can imagine this becoming an incubating module.

However, we are currently lacking the capacity to review and merge PRs for new modules, so I can't promise when we will be able to get to it. This does not mean, that we don't appreciate your contribution. We are currently in the process to rework our current approach around modules, to make it easier for the community to contribute them.

In case you are interested in getting it out to users as early as possible, you can always consider publishing it in your own repo (this is in no way an indication, that we don't want to add it to our repo).

eddumelendez

Hi @julb, sorry for the late reply and thanks for your contribution! The code looks good but I've added a few comments.

Are you willing to update the PR?

julb · 2022-07-28T13:27:23Z

Hey @eddumelendez
It is updated.
Thanks for your feedback!

eddumelendez

just a couple of comments and LGTM. Once again, thanks!

eddumelendez

few more comments from @kiview and me :)

julb · 2022-07-29T07:23:20Z

@eddumelendez @kiview all remarks addressed! Thanks a lot.

eddumelendez

LGTM

kiview

Thanks a lot for the PR @julb, LGTM 👍

There is one open question regarding the package name, but this is something we as maintainer should align on first 🙂

kiview · 2022-08-04T14:34:03Z

@@ -0,0 +1,103 @@
+package org.testcontainers.consul;


We normally use the org.testcontainers.containers package for Testcontainers container implementation, but AFAIK this makes Testcontainers suffer from the Split-Package problem when using the modulepath (as described here: https://stackoverflow.com/questions/53245628/jdk9-automatic-modules-and-split-packages-dependencies).

We also have other modules, that follow a package convention of org.testcontainers.{module-name}, the same as here. So I am inclined to say, we should leave it as provided in the PR.

WDYT @bsideup?

Although we have some AUTHORS files in different modules, this is generally not what we do and we prefer using the Git history to identify authors.

…ontainer.java

eddumelendez · 2022-08-04T20:11:20Z

@julb can you please add consul to

.github/ISSUE_TEMPLATE/bug_report.yaml
.github/ISSUE_TEMPLATE/enhancement.yaml
.github/ISSUE_TEMPLATE/feature.yaml
.github/dependabot.yml
.github/labeler.yml

Index: .github/ISSUE_TEMPLATE/bug_report.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/ISSUE_TEMPLATE/bug_report.yaml b/.github/ISSUE_TEMPLATE/bug_report.yaml
--- a/.github/ISSUE_TEMPLATE/bug_report.yaml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/ISSUE_TEMPLATE/bug_report.yaml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -17,6 +17,7 @@
         - Azure
         - Cassandra
         - Clickhouse
+        - Consul
         - Couchbase
         - DB2
         - Dynalite
Index: .github/ISSUE_TEMPLATE/enhancement.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/ISSUE_TEMPLATE/enhancement.yaml b/.github/ISSUE_TEMPLATE/enhancement.yaml
--- a/.github/ISSUE_TEMPLATE/enhancement.yaml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/ISSUE_TEMPLATE/enhancement.yaml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -17,6 +17,7 @@
         - Azure
         - Cassandra
         - Clickhouse
+        - Consul
         - Couchbase
         - DB2
         - Dynalite
Index: .github/ISSUE_TEMPLATE/feature.yaml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/ISSUE_TEMPLATE/feature.yaml b/.github/ISSUE_TEMPLATE/feature.yaml
--- a/.github/ISSUE_TEMPLATE/feature.yaml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/ISSUE_TEMPLATE/feature.yaml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -17,6 +17,7 @@
         - Azure
         - Cassandra
         - Clickhouse
+        - Consul
         - Couchbase
         - DB2
         - Dynalite
Index: .github/dependabot.yml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/dependabot.yml b/.github/dependabot.yml
--- a/.github/dependabot.yml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/dependabot.yml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -43,6 +43,11 @@
       interval: "monthly"
     open-pull-requests-limit: 10
   - package-ecosystem: "gradle"
+    directory: "/modules/consul"
+    schedule:
+      interval: "monthly"
+    open-pull-requests-limit: 10
+  - package-ecosystem: "gradle"
     directory: "/modules/couchbase"
     schedule:
       interval: "monthly"
Index: .github/labeler.yml
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/.github/labeler.yml b/.github/labeler.yml
--- a/.github/labeler.yml	(revision c9fc9afff3c13bdf7c6f7762ab8815ed91550399)
+++ b/.github/labeler.yml	(revision 95124487415f8ffad3a06df54d6081f4dd6e269d)
@@ -15,6 +15,8 @@
   - modules/clickhouse/*
 "modules/cockroachdb":
   - modules/cockroachdb/*
+"modules/consul":
+  - modules/consul/*
 "modules/couchbase":
   - modules/couchbase/*
 "modules/db2":

julb · 2022-08-05T07:19:16Z

@eddumelendez @kiview done!
Thanks for your reviews and your time.

kiview · 2022-08-05T08:17:23Z

Thanks @julb, we will merge once we internally decided on how to proceed with package names for container modules 🙂

eddumelendez

one last thing @julb ! We are not able to push to your branch so here we are again asking you for some changes 😬 about moving to assertj

Thank you so much!

eddumelendez · 2022-08-06T00:15:53Z

+        // Write operation
+        properties.forEach((key, value) -> {
+            Response<Boolean> writeResponse = consulClient.setKVValue(key, value);
+            Assertions.assertThat(writeResponse.getValue()).isTrue();


import static

eddumelendez · 2022-08-06T00:21:47Z

+    public void readFirstPropertyPathWithCli() throws IOException, InterruptedException {
+        GenericContainer.ExecResult result = consulContainer.execInContainer("consul", "kv", "get", "config/testing1");
+        final String output = result.getStdout().replaceAll("\\r?\\n", "");
+        MatcherAssert.assertThat(output, CoreMatchers.containsString("value123"));


move to assertThat from assertj, please

eddumelendez · 2022-08-06T00:25:03Z

+        // Read operation
+        properties.forEach((key, value) -> {
+            Response<GetValue> readResponse = consulClient.getKVValue(key);
+            Assertions.assertThat(readResponse.getValue().getDecodedValue()).isEqualTo(value);


import static

eddumelendez · 2022-08-06T00:26:05Z

+        RestAssured
+            .given()
+            .when()
+            .get("http://" + getHostAndPort() + "/v1/kv/config/testing1")
+            .then()
+            .assertThat()
+            .body(
+                "[0].Value",
+                CoreMatchers.equalTo(Base64.getEncoder().encodeToString("value123".getBytes(StandardCharsets.UTF_8)))
+            );


can we do something like

Suggested change

RestAssured

.given()

.when()

.get("http://" + getHostAndPort() + "/v1/kv/config/testing1")

.then()

.assertThat()

.body(

"[0].Value",

CoreMatchers.equalTo(Base64.getEncoder().encodeToString("value123".getBytes(StandardCharsets.UTF_8)))

);

io.restassured.response.Response response = RestAssured

.given()

.when()

.get("http://" + getHostAndPort() + "/v1/kv/config/testing1")

.andReturn();

assertThat(response.body().jsonPath().getString("[0].Value"))

.isEqualTo(Base64.getEncoder().encodeToString("value123".getBytes(StandardCharsets.UTF_8)));

eddumelendez · 2022-08-15T14:33:02Z

Thanks @julb ! PR has been merged and now we have a consul module!

julb · 2022-08-15T19:00:16Z

OMG!
Thanks a lot for your support, that's awesome !!
Looking forward for the official release!

julb requested review from bsideup, kiview and rnorth as code owners November 17, 2021 14:07

julb changed the title ~~Provide testcontainers consul module~~ New module: consul Nov 17, 2021

kiview added the type/new module label Nov 17, 2021

eddumelendez requested changes Jul 26, 2022

View reviewed changes

eddumelendez assigned kiview and eddumelendez Jul 26, 2022

julb requested a review from a team July 28, 2022 13:01

github-actions Bot added the type/docs label Jul 28, 2022

eddumelendez reviewed Jul 28, 2022

View reviewed changes

Comment thread modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java

Comment thread modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java Outdated

eddumelendez reviewed Jul 28, 2022

View reviewed changes

Comment thread modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java Outdated

eddumelendez requested changes Jul 28, 2022

View reviewed changes

julb requested a review from eddumelendez July 29, 2022 07:24

eddumelendez reviewed Jul 29, 2022

View reviewed changes

Comment thread modules/consul/src/test/java/org/testcontainers/consul/ConsulContainerTest.java

eddumelendez added this to the next milestone Jul 29, 2022

julb requested a review from eddumelendez August 3, 2022 09:37

eddumelendez approved these changes Aug 3, 2022

View reviewed changes

kiview reviewed Aug 4, 2022

View reviewed changes

Comment thread modules/consul/src/main/java/org/testcontainers/consul/ConsulContainer.java Outdated

kiview approved these changes Aug 4, 2022

View reviewed changes

julb and others added 7 commits August 4, 2022 17:11

Provide testcontainers consul module

9e8b9d1

Take PR feedback into account for Consul module

bcc0ed5

consul-module: take additional remarks into account

aa24a61

consul-module: fix checkstyle issues

36536f1

Delete AUTHORS file

02fa7bf

Although we have some AUTHORS files in different modules, this is generally not what we do and we prefer using the Git history to identify authors.

Update modules/consul/src/main/java/org/testcontainers/consul/ConsulC…

951c270

…ontainer.java

consul: reference consul in github config files

8fe111f

eddumelendez reviewed Aug 6, 2022

View reviewed changes

eddumelendez changed the title ~~New module: consul~~ Add Consul module Aug 6, 2022

eddumelendez merged commit 625947f into testcontainers:master Aug 15, 2022

eddumelendez mentioned this pull request Aug 31, 2022

Add module for Hashicorp Consul #4860

Closed

Conversation

julb commented Nov 17, 2021

Uh oh!

kiview commented Nov 17, 2021

Uh oh!

eddumelendez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julb commented Jul 28, 2022

Uh oh!

eddumelendez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eddumelendez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

julb commented Jul 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

eddumelendez left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kiview left a comment

Choose a reason for hiding this comment

Uh oh!

kiview Aug 4, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eddumelendez commented Aug 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

julb commented Aug 5, 2022

Uh oh!

kiview commented Aug 5, 2022

Uh oh!

eddumelendez left a comment

Choose a reason for hiding this comment

Uh oh!

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

Uh oh!

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

Uh oh!

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

Uh oh!

eddumelendez Aug 6, 2022

Choose a reason for hiding this comment

Uh oh!

eddumelendez commented Aug 15, 2022

Uh oh!

julb commented Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

julb commented Jul 29, 2022 •

edited

Loading

eddumelendez commented Aug 4, 2022 •

edited

Loading

julb commented Aug 15, 2022 •

edited

Loading