aai-institute / jobq

https://aai-institute.github.io/jobq/latest
Apache License 2.0
1 stars 1 forks source link

(docs) Add declarative yaml explanations #129

Closed maxmynter closed 3 weeks ago

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 82.00000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 56.60%. Comparing base (4b383b3) to head (8b19bf6). Report is 59 commits behind head on main.

:white_check_mark: All tests successful. No failed tests found.

Files with missing lines Patch % Lines
client/src/jobq/assembler/config.py 79.26% 17 Missing :warning:
client/src/jobq/assembler/renderers.py 94.44% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #129 +/- ## ========================================== + Coverage 56.42% 56.60% +0.18% ========================================== Files 61 62 +1 Lines 2997 3132 +135 ========================================== + Hits 1691 1773 +82 - Misses 1306 1359 +53 ``` | [Flag](https://app.codecov.io/gh/aai-institute/jobq/pull/129/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aai-institute) | Coverage Δ | | |---|---|---| | [backend](https://app.codecov.io/gh/aai-institute/jobq/pull/129/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aai-institute) | `88.27% <ø> (ø)` | | | [client](https://app.codecov.io/gh/aai-institute/jobq/pull/129/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aai-institute) | `48.62% <82.00%> (+0.69%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=aai-institute#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

AdrianoKF commented 1 month ago

I have pushed a rather large refactoring in d98d73836935ecb1269b52445f543452e18042b3:

This addresses most of my review comments from above but breaks our example files. If we decide to keep these changes, we will have to adapt those files.

Additionally, we might consider allowing both list and map formats for env/labels/build args to mirror the Docker Compose syntax:

env:
  FOO: val
  BAR: val

# or

env:
  - FOO=val
  - BAR=val

# But not, as we had before

env:
  - FOO: val  # (!)
maxmynter commented 4 weeks ago

I have one open issue of which I haven't yet found the cause.

When I submit the example-hello.py job it builds successfully but then fails because the docker dependency is missing even though that is only used for type checking and guarded by typing.TYPE_CHECKING.

So we need to figure out why the execution on the server attempts type checking for that example.

cc: @AdrianoKF

AdrianoKF commented 4 weeks ago

I have one open issue of which I haven't yet found the cause.

I wasn't able to reproduce that error, but another one: in the client's pyproject.toml, the jobs_execute script was still pointing to the jobs.execute (not jobq), which prevented the container from finding the script.

Fixed it in e6c696d694aed840d695a15f09c460a60b804d6a, now the example works fine on my machine.

maxmynter commented 3 weeks ago

This fix also resolved the typing issue mentioned above, even though i don't fully understand how were related.

I'll squash all changes to the docs and then rebase such that we have history of the features/fixes in our main commit log.