fix: removed hardcoding from docker-image dir and kinds #728
Conversation
ahal
left a comment
ahal
left a comment
There was a problem hiding this comment.
Nice, I like it!
The CI is broken though due to the dash vs underscore mismatch. You can likely reproduce locally with taskgraph full
src/taskgraph/config.py
Outdated
| Required("trust-domain"): str, | ||
| Optional( | ||
| "docker-image-kind", | ||
| description="What kind of docker image to use for the task.", |
There was a problem hiding this comment.
How about:
Name of the docker image kind (default: docker-image)
ahal
left a comment
ahal
left a comment
There was a problem hiding this comment.
This looks good!
There are some test failures though, probably just need to update some of them for the new function signature. You should be able to reproduce locally with uv run pytest
taskcluster/config.yml
Outdated
|
|
||
| task-priority: low | ||
|
|
||
| docker-image-kind: docker-image |
There was a problem hiding this comment.
I would lean towards not defining this unless we actually want to use a non-default name, but doesn't really matter I guess.
|
I updated the where the default - docker-image-kind is decide as the optional + default combinations doesn't work as schema(obj) is not called on the graph config schema so the fix I had was to add a try catch field in images_path as that is only place it would be called |
ahal
left a comment
ahal
left a comment
There was a problem hiding this comment.
Thanks. A couple minor suggestions, but looks good!
abhishekmadan30
force-pushed
the
issue-21
branch
from
bc2c5ea to
37fc2d6
Compare
abhishekmadan30
force-pushed
the
issue-21
branch
from
37fc2d6 to
ef11bfc
Compare
2 participants
Closes #21