Resource limits now also from services key; Added tests #1173
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 file(s) reviewed, 11 comment(s)
Edit PR Review Bot Settings | Greptile
|
Eco CI Output:
🌳 CO2 Data: Total cost of whole PR so far: |
ArneTR
added a commit
that referenced
this pull request
May 4, 2025
* main: (27 commits) Bump orjson from 3.10.16 to 3.10.18 (#1168) Bump pydantic from 2.11.3 to 2.11.4 (#1169) Bump psycopg[binary] from 3.2.6 to 3.2.7 (#1171) Resource limits now also from services key; Added tests (#1173) (fix): Cron job queue check logic was reversed Updated ee Moving [system] and [machine] to upper case Shortened model Watchlist must insert usage_scenario_variables Comment for MCP Job ID now a field to filter by Usage Scenario Variables and ScenarioRunner templates (#1172) Disabled providers are now removed also from DB entry not only from effective measurement (fix): Entries for config options that were null where not correctly showing Made temperatur error more helpful Typo Root DIR of GMT was not accurate Renamed Runner to ScenarioRunner and moved to lib/ Software add now returns job_id on insert; Jobs API now allows filter for job_id [skip ci] (#1170) (test-fix): New wording ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Greptile Summary
Added comprehensive testing and validation for Docker container resource limits, ensuring consistent configuration between service-level and deploy.resources.limits specifications.
This PR reworks how GMT handles Resource Limits.
Previously this was done through the deploy key in services.
However this key is intended for Docker Swarm.
GMT now also support the cpus and mem_limit keys under services.
When using Docker Compose directly the deploy settings for CPU and Memory have no further implications and will just overwrite any settings given via services top level keys. Implementation is here: https://github.com/docker/compose/blob/5bb46035cf12e5b2fc2f3b09522a392b7b753120/pkg/compose/create.go#L713
lib/schema_checker.pyto enforce memory limits as strings and CPU limits as string/float/inttests/test_resource_limits.pyto verify matching CPU/memory limits between deploy and service-level configslib/scenario_runner.pyto properly handle and apply resource limits from both configuration locationstests/data/usage_scenarios/for various resource limit scenarios including invalid configurations