Add compose stack name + allow test stack to run in parallel #1093

davidkopp · 2025-03-11T14:24:30Z

When running the containers of GMT in Docker Desktop the stack was called "docker" because of the base path. This PR adds a proper stack name: green-metrics-tool.

Docs:

One use case of having different stacks is to have different environments. So I thought it could make sense to allow to run the test environment in parallel to the production/development environment.

Greptile Summary

This PR adds a proper stack name to Docker Compose configurations and enables parallel execution of test and production environments by modifying port configurations and service names.

Added stack name 'green-metrics-tool' in /docker/compose.yml.example for better organization in Docker Desktop
Changed PostgreSQL port from 9573 to 9574 in test configurations to allow parallel environment execution
Modified /tests/setup-test-env.py to handle new port mappings for nginx (9143) and postgres (9574)
Updated /tests/test_functions.py to use dynamic database configuration from GlobalConfig instead of hardcoded values
Added 'test-' prefix to volume and service names in test environment for clear separation

_{💡 (4/5) You can add custom instructions or style guidelines for the bot here!}

ArneTR · 2025-03-12T14:03:41Z

Looks like a good idea to me. @ribalba can you test this please? Did only look at time bc sickness

ArneTR · 2025-03-12T14:03:48Z

@greptileai

greptile-apps

_{5 file(s) reviewed, 2 comment(s)}
_{Edit PR Review Bot Settings | Greptile}

greptile-apps · 2025-03-12T14:04:25Z

tests/setup-test-env.py

+TEST_NGINX_PORT_MAPPING = [f"{TEST_NGINX_PORT}:{BASE_NGINX_PORT}"] # only change public port
+BASE_DATABASE_PORT = 9573
+TEST_DATABASE_PORT = 9574
+TEST_DATABASE_PORT_MAPPING = [f"{TEST_DATABASE_PORT}:{TEST_DATABASE_PORT}"] # change external and internal port


logic: Port mapping sets both external and internal ports to TEST_DATABASE_PORT, but comment suggests only external port should change. This could cause connectivity issues.

Suggested change

TEST_DATABASE_PORT_MAPPING = [f"{TEST_DATABASE_PORT}:{TEST_DATABASE_PORT}"] # change external and internal port

TEST_DATABASE_PORT_MAPPING = [f"{TEST_DATABASE_PORT}:{BASE_DATABASE_PORT}"] # change only external port

greptile-apps · 2025-03-12T14:04:26Z

tests/setup-test-env.py

+        # Edit stack name
+        compose['name'] = 'green-metrics-tool-test'


style: Stack name is set inside service loop, causing it to be overwritten multiple times. Move outside the loop to improve clarity and prevent potential issues.

ribalba · 2025-03-12T15:59:11Z

I like the idea of changing the port. The name is also changed in docker desktop. See screenshot.

The only thing is I am getting quite a few test errors because of the port change. @davidkopp can you see them? Otherwise I can send them to you.

davidkopp · 2025-03-12T21:27:32Z

I forgot to check if the frontend tests are still working. Sorry for that.
I have now added some fixes so the frontend tests work again. For that I needed a workaround, see my change in tests/start-test-containers.sh. Hope that's ok. I didn't wanted to duplicate the frontend config, so I came up with this workaround to temporarily change the ports only during the start of the containers.

davidkopp · 2025-03-13T09:51:33Z

My hack with temporarily changing the ports in ../frontend/js/helpers/config.js had some issues (e.g. when docker compose is started in the default attached mode, the revert command is executed after the compose stack is shutdown).

I have now reverted the hack and instead make a copy of the frontend config file similar how config.yml is overwritten in the containers.

ArneTR · 2025-03-13T16:54:02Z

LGtM, lets run the tests!

ArneTR · 2025-03-13T17:27:02Z

Test seem to work fine. Did now also a code review. Here my remakrs:

Greptile mentioned that the stack name is set in the loop. I think that is unnecessary, or? If so, can you move it outside the service loop
You are setting the DB to a different port for the tests inside the containers and then map TEST_DATABASE_PORT:TEST_DATABASE_PORT. To my understanding this makes it work by accident. I would argue for leaving the DB internally at 9573 and just changing the mapping
When I try to have the test setup and the live setup running locally I get a conflict. Redis is not handled atm. Was that intended?

davidkopp · 2025-03-13T21:01:22Z

Thanks for your remarks.

Greptile was right. I moved the set of the stack name now outside of the loop
The port mapping of the database TEST_DATABASE_PORT:TEST_DATABASE_PORT is on purpose. Otherwise it wouldn't work at the moment. There are 2 different ways how to database is accessed: DB.py (public port) and test_functions.py with psql inside the container (internal port). However in config.yml / test-config.yml only one port is specified. If I change the port in the config both, external and internal port has to be changed. Another way would be to split the config into two ports, but I think that would be more confusing.
The redis conflict was not intended. Somehow I didn't had a redis conflict yesterday. The port mapping was just missing in test-compose.yml (don't know why). Know I have fixed it.
btw: Also here I needed to change the external and internal port (TEST_REDIS_PORT:TEST_REDIS_PORT). I'm not sure why I have to change the internal port, but otherwise it didn't work.

With the redis related change a re-install of GMT is required so the port is in config.yml. If this is a problem, the default port 6379 could be specified as a fall back in the Python code if none is given.

ArneTR · 2025-03-14T08:39:05Z

Code looks good and works well on my local box. Running the tests now!

One thing that should be considered: It is still not possible to run the tests AND a benchmarking run in parallel as the providers will conflict each other. I however also see no valid reason to do that, other than some next level hyper parallel development. If that is needed we typically outsource the test to GitHub to run in parallel.

ArneTR · 2025-03-14T09:00:07Z

Thanks for the PR ❤️. merged.

…oding-solutions#1093) * Add compose stack name * Allow test stack to run in parallel * Fix frontend tests * Minor * Fix access of the frontend to the test nginx container * Ensure to revert config change on exit * Duplicate frontend config instead of temporarily changing it * Fix set test compose name out of loop * Fix redis connection

* Add support for entrypoint attribute * Fix entrypoint implementation * Improve test case * Add test case with script as entrypoint * Add test case for entrypoint in conjunction with command * Bump deepdiff from 8.2.0 to 8.3.0 (#1087) Bumps [deepdiff](https://github.com/seperman/deepdiff) from 8.2.0 to 8.3.0. - [Release notes](https://github.com/seperman/deepdiff/releases) - [Changelog](https://github.com/seperman/deepdiff/blob/master/docs/changelog.rst) - [Commits](seperman/deepdiff@8.2.0...8.3.0) --- updated-dependencies: - dependency-name: deepdiff dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * E-Mail now made optional * Checking int fields for params and limiting retrieval amount (#1089) * Add compose stack name + allow test stack to run in parallel (#1093) * Add compose stack name * Allow test stack to run in parallel * Fix frontend tests * Minor * Fix access of the frontend to the test nginx container * Ensure to revert config change on exit * Duplicate frontend config instead of temporarily changing it * Fix set test compose name out of loop * Fix redis connection * Allow empty entrypoint -> default entrypoint is ignored * Minor test improvement * Add attribute "clear-entrypoint" * Revert "Add attribute "clear-entrypoint"" This reverts commit 001b3ec. * Remove test entrypoint with single word (not relevant for practice) * Replace usage of 'yes' with 'tail' in test case --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Arne Tarara <[email protected]>

* Add compose stack name * Allow test stack to run in parallel * Fix frontend tests * Minor * Fix access of the frontend to the test nginx container * Ensure to revert config change on exit * Duplicate frontend config instead of temporarily changing it * Fix set test compose name out of loop * Fix redis connection

* Add support for entrypoint attribute * Fix entrypoint implementation * Improve test case * Add test case with script as entrypoint * Add test case for entrypoint in conjunction with command * Bump deepdiff from 8.2.0 to 8.3.0 (#1087) Bumps [deepdiff](https://github.com/seperman/deepdiff) from 8.2.0 to 8.3.0. - [Release notes](https://github.com/seperman/deepdiff/releases) - [Changelog](https://github.com/seperman/deepdiff/blob/master/docs/changelog.rst) - [Commits](seperman/deepdiff@8.2.0...8.3.0) --- updated-dependencies: - dependency-name: deepdiff dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * E-Mail now made optional * Checking int fields for params and limiting retrieval amount (#1089) * Add compose stack name + allow test stack to run in parallel (#1093) * Add compose stack name * Allow test stack to run in parallel * Fix frontend tests * Minor * Fix access of the frontend to the test nginx container * Ensure to revert config change on exit * Duplicate frontend config instead of temporarily changing it * Fix set test compose name out of loop * Fix redis connection * Allow empty entrypoint -> default entrypoint is ignored * Minor test improvement * Add attribute "clear-entrypoint" * Revert "Add attribute "clear-entrypoint"" This reverts commit 001b3ec. * Remove test entrypoint with single word (not relevant for practice) * Replace usage of 'yes' with 'tail' in test case --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Arne Tarara <[email protected]>

* main: Reverted XGBoost upgrade as GitHub Actions fails Bump aiohttp from 3.11.13 to 3.11.14 (#1098) Bump deepdiff from 8.3.0 to 8.4.2 (#1102) HealthCheck JSON errors are now scraped (#1103) Updated Cloud Energy Adding client max body size 3M More clear about GMT free version limit Logs previous exception if present (#1097) Moving back to pull_request as we are not getting protetction for synchronize pushes Add support for entrypoint attribute (#1088) Add compose stack name + allow test stack to run in parallel (#1093) Moving to pull_request_target to have secrets available even in external contributed PRs Bump psycopg[binary] from 3.2.5 to 3.2.6 (#1095)

* (Frontend): Energy-ID is now modular frontend component; Home page draft; Refactoring of config.js [skip ci] * Added sample display of cards for connected tools [skip ci] * h1 header change repositories / last 50 runs * Normalized namings; Added /insigts endpoint * Changing name to MetricRunner * Updated meta * Reverted metric-runner URL naming * Modularized install script incl. help; Segmented out MetricRunner API * Making metric runner main item also link to runs * REplace CarbonDB and PowerHOG always * More uniform style * Bugfix for import * Update README.md Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> * Removed duplicate statement * Updated EE * Bump psycopg[binary] from 3.2.5 to 3.2.6 (#1095) Bumps [psycopg[binary]](https://github.com/psycopg/psycopg) from 3.2.5 to 3.2.6. - [Changelog](https://github.com/psycopg/psycopg/blob/master/docs/news.rst) - [Commits](psycopg/psycopg@3.2.5...3.2.6) --- updated-dependencies: - dependency-name: psycopg[binary] dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * Moving to pull_request_target to have secrets available even in external contributed PRs * Add compose stack name + allow test stack to run in parallel (#1093) * Add compose stack name * Allow test stack to run in parallel * Fix frontend tests * Minor * Fix access of the frontend to the test nginx container * Ensure to revert config change on exit * Duplicate frontend config instead of temporarily changing it * Fix set test compose name out of loop * Fix redis connection * Add support for entrypoint attribute (#1088) * Add support for entrypoint attribute * Fix entrypoint implementation * Improve test case * Add test case with script as entrypoint * Add test case for entrypoint in conjunction with command * Bump deepdiff from 8.2.0 to 8.3.0 (#1087) Bumps [deepdiff](https://github.com/seperman/deepdiff) from 8.2.0 to 8.3.0. - [Release notes](https://github.com/seperman/deepdiff/releases) - [Changelog](https://github.com/seperman/deepdiff/blob/master/docs/changelog.rst) - [Commits](seperman/deepdiff@8.2.0...8.3.0) --- updated-dependencies: - dependency-name: deepdiff dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> * E-Mail now made optional * Checking int fields for params and limiting retrieval amount (#1089) * Add compose stack name + allow test stack to run in parallel (#1093) * Add compose stack name * Allow test stack to run in parallel * Fix frontend tests * Minor * Fix access of the frontend to the test nginx container * Ensure to revert config change on exit * Duplicate frontend config instead of temporarily changing it * Fix set test compose name out of loop * Fix redis connection * Allow empty entrypoint -> default entrypoint is ignored * Minor test improvement * Add attribute "clear-entrypoint" * Revert "Add attribute "clear-entrypoint"" This reverts commit 001b3ec. * Remove test entrypoint with single word (not relevant for practice) * Replace usage of 'yes' with 'tail' in test case --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Arne Tarara <[email protected]> * Unified display of home page blocks * Typos and hardening * Updated EE * Test-Fix * Renamed MetricRunner to ScenarioRunner * Activate scenario runner not on every input, but only empty and Y/y * Typo * Removed additional info * Bugfix: Inverted logic * File renames * Added insights tests * Test fix * Test install routine activates components always. No matter what other local config is * Removing file from repo * Typo --------- Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: David Kopp <[email protected]>

davidkopp added 2 commits March 11, 2025 15:19

Add compose stack name

6cffc43

Allow test stack to run in parallel

b8f85f6

greptile-apps bot reviewed Mar 12, 2025

View reviewed changes

davidkopp added 3 commits March 12, 2025 21:39

Fix frontend tests

9797708

Minor

88729c1

Fix access of the frontend to the test nginx container

1486b2b

Ensure to revert config change on exit

ed5d559

davidkopp marked this pull request as draft March 13, 2025 09:22

Duplicate frontend config instead of temporarily changing it

cd0d94f

davidkopp marked this pull request as ready for review March 13, 2025 09:51

Merge branch 'main' into add-compose-stack-name

c511093

davidkopp and others added 3 commits March 13, 2025 20:06

Merge branch 'main' into add-compose-stack-name

10336c6

Fix set test compose name out of loop

673adda

Fix redis connection

cf9375e

ArneTR merged commit 6931943 into green-coding-solutions:main Mar 14, 2025
1 check passed

davidkopp deleted the add-compose-stack-name branch March 14, 2025 11:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add compose stack name + allow test stack to run in parallel #1093

Add compose stack name + allow test stack to run in parallel #1093

Uh oh!

davidkopp commented Mar 11, 2025 •

edited by greptile-apps bot

Loading

Uh oh!

ArneTR commented Mar 12, 2025

Uh oh!

ArneTR commented Mar 12, 2025

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Mar 12, 2025

Uh oh!

greptile-apps bot Mar 12, 2025

Uh oh!

ribalba commented Mar 12, 2025 •

edited

Loading

Uh oh!

davidkopp commented Mar 12, 2025

Uh oh!

davidkopp commented Mar 13, 2025

Uh oh!

ArneTR commented Mar 13, 2025

Uh oh!

ArneTR commented Mar 13, 2025

Uh oh!

davidkopp commented Mar 13, 2025

Uh oh!

ArneTR commented Mar 14, 2025

Uh oh!

ArneTR commented Mar 14, 2025

Uh oh!

Uh oh!

Uh oh!

	TEST_DATABASE_PORT_MAPPING = [f"{TEST_DATABASE_PORT}:{TEST_DATABASE_PORT}"] # change external and internal port
	TEST_DATABASE_PORT_MAPPING = [f"{TEST_DATABASE_PORT}:{BASE_DATABASE_PORT}"] # change only external port

		# Edit stack name
		compose['name'] = 'green-metrics-tool-test'

Add compose stack name + allow test stack to run in parallel #1093

Add compose stack name + allow test stack to run in parallel #1093

Uh oh!

Conversation

davidkopp commented Mar 11, 2025 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Uh oh!

ArneTR commented Mar 12, 2025

Uh oh!

ArneTR commented Mar 12, 2025

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Mar 12, 2025

Choose a reason for hiding this comment

Uh oh!

ribalba commented Mar 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davidkopp commented Mar 12, 2025

Uh oh!

davidkopp commented Mar 13, 2025

Uh oh!

ArneTR commented Mar 13, 2025

Uh oh!

ArneTR commented Mar 13, 2025

Uh oh!

davidkopp commented Mar 13, 2025

Uh oh!

ArneTR commented Mar 14, 2025

Uh oh!

ArneTR commented Mar 14, 2025

Uh oh!

Uh oh!

Uh oh!

davidkopp commented Mar 11, 2025 •

edited by greptile-apps bot

Loading

ribalba commented Mar 12, 2025 •

edited

Loading