Skip to content

fix: properly read docker task config from service props & task props #1616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 10, 2025

Conversation

0utplay
Copy link
Member

@0utplay 0utplay commented May 5, 2025

Motivation

In #1251 an issue regarding docker configs was resolved. The docker configuration was only read from the service props which is not used by default. The properties are only present in the task and not the service. This PR now fixes the issue and brings support to resolve the config from service & task properties.

Modification

Added a three step process to figure out which configuration to use when resolving properties that might be task / service specific.

The order is now:
Service -> Task (if present) -> Module Config

Result

The configuration is read correctly

@0utplay 0utplay added v: 4.X This pull should be included in the 4.0 release t: fix A pull request introducing a fix for a bug. in: module An issue/pull request releated to one of the internal modules labels May 5, 2025
@0utplay 0utplay requested a review from derklaro May 5, 2025 19:09
@0utplay 0utplay self-assigned this May 5, 2025
@0utplay 0utplay marked this pull request as ready for review May 5, 2025 19:23
@0utplay
Copy link
Member Author

0utplay commented May 5, 2025

This PR works in a way that if the config is present in the service or the task, then all values are taken from that config. No merging of e.g. exposed ports is done. This might need to be discussed

Copy link

github-actions bot commented May 7, 2025

Test Results

 49 files  ±0   49 suites  ±0   1m 27s ⏱️ -17s
439 tests ±0  439 ✅ ±0  0 💤 ±0  0 ❌ ±0 
770 runs  ±0  770 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a98e65f. ± Comparison against base commit 29ee9ff.

This pull request removes 38 and adds 38 tests. Note that renamed tests count towards both.
eu.cloudnetservice.driver.impl.document.DocumentSerialisationTest ‑ [4] {"b":1,"s":2,"i":3,"l":4,"f":5.0,"d":6.0,"c":"/","string":"Hello, World!","bol":true,"cloud":["Ben?","Yes","No","HoHoHoHo"],"world":{"insane":"!","hello":"world","this":"is"}}, PRETTY
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [14] 2025-04-24
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [23] 15:41:06.896625480
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [28] 15:41:06.896802781Z
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [29] 15:41:06.896827718Z
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [30] 15:41:06.896851492+05:00
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [31] 15:41:06.896866029-03:00
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [34] 2025-04-24T15:41:06.897291204
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [39] 2025-04-24T15:41:06.897816807Z
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [4] 2025-04-24T15:41:06.891832980Z
…
eu.cloudnetservice.driver.impl.document.DocumentSerialisationTest ‑ [4] {"b":1,"s":2,"i":3,"l":4,"f":5.0,"d":6.0,"c":"/","string":"Hello, World!","bol":true,"cloud":["Ben?","Yes","No","HoHoHoHo"],"world":{"insane":"!","this":"is","hello":"world"}}, PRETTY
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [14] 2025-05-07
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [23] 16:16:00.035582606
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [28] 16:16:00.035716636Z
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [29] 16:16:00.035740100Z
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [30] 16:16:00.035762562+05:00
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [31] 16:16:00.035778261-03:00
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [34] 2025-05-07T16:16:00.035911801
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [39] 2025-05-07T16:16:00.036053465Z
eu.cloudnetservice.driver.impl.document.gson.JavaTimeSerializerTest ‑ [4] 2025-05-07T16:16:00.030748239Z
…

@0utplay 0utplay merged commit a081d41 into nightly May 10, 2025
5 checks passed
@0utplay 0utplay deleted the fix/docker-task-config branch May 10, 2025 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: module An issue/pull request releated to one of the internal modules t: fix A pull request introducing a fix for a bug. v: 4.X This pull should be included in the 4.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants