[exec] Fix: use --config-file override in test command

[denis-angilella]

klioexec test --config-file option was ignored

I have included unit tests

Note that integration tests are failing due to a missing secret, this is not related to changes in this PR

Checklist for PR author(s)

• Format the pull request title like [cli] Fixes bugs in 'klio job fake-cmd'.
• Changes are covered by unit tests (no major decrease in code coverage %) and/or integration tests.
• already documented but not working / Document any relevant additions/changes in the appropriate spot in docs/src.
• For any change that affects users, update the package's changelog in docs/src/reference/<package>/changelog.rst.

[CLAassistant]

All committers have signed the CLA.

[denis-angilella]

@DanSimon @econchick @fallonchen Hi, may I ask for a review of this PR please?

[fallonchen]

@denis-angilella Thanks for contributing! Taking a look now :)

[fallonchen]

@denis-angilella The changes here are effective for klioexec test. However, if you want to run klio job test --config-file klio-job-custom.yaml, additional changes need to be made within the test command in the cli module, as the job test subcommand in klio cli is not configured to materialize the runtime configuration and pass it to the container that runs the tests.

Do you need the klio job test command to work with the config-file flag, or just the klioexec test command?

[denis-angilella]

Hi @fallonchen, thanks for your comment!

Do you need the klio job test command to work with the config-file flag, or just the klioexec test command?

At the moment we are using klioexec directly due to limitations in klio cli (see #175 and #176)

if you want to run klio job test --config-file klio-job-custom.yaml, additional changes need to be made within the test command in the cli module, as the job test subcommand in klio cli is not configured to materialize the runtime configuration and pass it to the container that runs the tests.

Thanks for pointing this out.

I'm confused: what is the --config-file option of klio job test used for if it is not forwarded to klioexec test?

Also, would requires_config_file = True be enough to transmit the materialized config file to klioexec via klio job test? I can add this change to this PR if it makes sense.

[fallonchen]

I'm confused: what is the --config-file option of klio job test used for if it is not forwarded to klioexec test?

Sorry for the confusion! Technically, the --config-file option of klio job test isn't directly forwarded. When config file materialization is enabled, the file at --config-file is combined with any runtime arguments to form the materialized config file /usr/src/config/materialized_config.yaml , and klio cli would run klioexec test --config-file /usr/src/config/materialized_config.yaml inside the container.

However, as you discovered, klioexec test doesn't respect the --config-file option, and config file materialization was not enabled for klio job test. I'm not sure why, but I think it was most likely an oversight of ours.

Hope that explanation makes sense!

Also, would requires_config_file = True be enough to transmit the materialized config file to klioexec via klio job test? I can add this change to this PR if it makes sense.

I've been looking into this a little bit - there's one additional change to be made. I can make a PR for it tonight, no need to add it to this one.

Thanks again for your contribution!

[fallonchen]

🚀 Thanks!

[denis-angilella]

When config file materialization is enabled, the file at --config-file is combined with any runtime arguments to form the materialized config file /usr/src/config/materialized_config.yaml , and klio cli would run klioexec test --config-file /usr/src/config/materialized_config.yaml inside the container.

However, as you discovered, klioexec test doesn't respect the --config-file option, and config file materialization was not enabled for klio job test. I'm not sure why, but I think it was most likely an oversight of ours.

@fallonchen thanks for the very clear explanation and follow up PR to add the missing part in klio cli!