valohai / valohai-utils

Python helper library for Valohai
MIT License
2 stars 2 forks source link

Updating valohai.yaml with valohai.prepare does not change the file name #118

Closed noorula closed 10 months ago

noorula commented 1 year ago

If the valohai.prepare has been run in myfirstfile.py to create a step called mystep, updating the step from another file, e.g. mysecondfile.py, does not update the filename in the command section.

I.e. even after running vh yaml step mysecondfile.py the command section in valohai.yaml looks like this:

    command:
    - pip install -r requirements.txt
    - python ./myfirstfile.py {parameters}
drazendee commented 1 year ago

(related to Zendesk ticket #1742)

hylje commented 1 year ago

valohai.prepare does not set the command. The file name to execute is not an explicit part of the yaml step configuration, either: you can write whatever list of commands you want as the yaml command field. We do not and should never modify the existing command definition while executing yaml step.

To address this issue, we should instead change the default command to use a parameter to choose what file it will execute, and yaml step will set the filename it is parsing as a new implicit (Python wise, explicit yaml wise) parameter.

Existing steps will behave as before: the special filename parameter only gets created or updated if it already exists on the original. This is important for backwards compatibility.

hylje commented 1 year ago

PRs in path dependency order; each step needs to be done prior to the next:

Problem: Roi needs to support the new yaml and utils prior to CLI getting them in order to interpolate commands for executions, but CLI does not pin yaml and utils dependencies so users installing CLI will get too new yaml and utils that Roi doesn't support yet.

Solutions:

hylje commented 1 year ago

testing with everything upgraded on local... (cli, roi)

Image

hylje commented 1 year ago

~Also, while additional fields don't break older versions of yaml/utils/cli, using old versions will drop the source-path parameter and the step will break for executions due to referring to the special option that no longer exists.~

~However, this is easily noticed or reverted as all changes should be in git. This does involve some customer awareness though.~

Nevermind, old versions will refuse to read a valohai.yaml with extra fields newly added to the schema.

We should probably add a valohai.yaml version field to the schema, allowing older versions of the CLI to notice that they're out of their depth modifying the file and provide a helpful error message.