Skip to content

Conversation

vy
Copy link
Member

@vy vy commented Sep 15, 2024

See #2953 for the 2.x port.

@vy vy added tests Pull requests or issues related to tests layouts Affects one or more Layout plugins labels Sep 15, 2024
@vy vy self-assigned this Sep 15, 2024
@ppkarwasz
Copy link
Contributor

I am wondering about a couple of things:

  • Should we activate the docker profile by default on machines that have /var/run/docker.sock? Or maybe we should ask the reviewers of a release to activate docker in their Maven settings if they have Docker installed?
  • The Docker image versions in these tests are somehow dated, but unfortunately Elastic Search does not provide a latest tag. Should we add org.elastisearch:elasticsearch to the dependency management of this module, so Dependabot can update the version regularly? My guess is that the Docker images are published shortly after the elasticsearch artifact is out.

@vy
Copy link
Member Author

vy commented Sep 16, 2024

Should we activate the docker profile by default on machines that have /var/run/docker.sock?

Bootstrapping an ELK stack is a pretty resource intensive process, I doubt if it will succeed without an OOM on every developer machine. Hence, I am happy with the current setup.

Or maybe we should ask the reviewers of a release to activate docker in their Maven settings if they have Docker installed?

I don't think we should, but I will update the review kit to reflect on the docker Maven profile – reviewers can choose for themselves.

  • The Docker image versions in these tests are somehow dated, but unfortunately Elastic Search does not provide a latest tag. Should we add org.elastisearch:elasticsearch to the dependency management of this module, so Dependabot can update the version regularly? My guess is that the Docker images are published shortly after the elasticsearch artifact is out.

Addressed this issue.

@ppkarwasz
Copy link
Contributor

FYI: I added:

<settings xmlns="http://maven.apache.org/SETTINGS/1.0.0"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="
              http://maven.apache.org/SETTINGS/1.0.0
              https://maven.apache.org/xsd/settings-1.0.0.xsd">
  <!-- Enable Docker profiles in Log4j tests -->
  <activeProfiles>
    <activeProfile>docker</activeProfile>
  </activeProfiles>
</settings>

We can add it to BUILDING.adoc as a copy-paste example.

Copy link
Contributor

@ppkarwasz ppkarwasz left a comment

Choose a reason for hiding this comment

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

LGTM

@vy vy merged commit 8d572fc into main Sep 17, 2024
9 checks passed
@vy vy deleted the feature/main/jtl-docker branch September 17, 2024 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
layouts Affects one or more Layout plugins tests Pull requests or issues related to tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants