Skip to content
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

Task.run wait for inputs #162

Merged
merged 1 commit into from
Dec 15, 2023
Merged

Task.run wait for inputs #162

merged 1 commit into from
Dec 15, 2023

Conversation

albertz
Copy link
Member

@albertz albertz commented Dec 15, 2023

New options:

  • WAIT_PERIOD_FOR_INPUTS_AVAILABLE
  • TASK_INPUTS_MUST_BE_AVAILABLE

Fix #159.

Note that TASK_INPUTS_MUST_BE_AVAILABLE is True by default, which can potentially break some old setups, where jobs used an input which was not an existing file but only the prefix of it (e.g. checkpoint file name).

New options:

WAIT_PERIOD_FOR_INPUTS_AVAILABLE
TASK_INPUTS_MUST_BE_AVAILABLE

Fix #159
@albertz albertz marked this pull request as ready for review December 15, 2023 10:58
@michelwi
Copy link
Contributor

Note that TASK_INPUTS_MUST_BE_AVAILABLE is True by default, which can potentially break some old setups, where jobs used an input which was not an existing file but only the prefix of it (e.g. checkpoint file name).

I personally am fine with the new default behavior. But I would like to check with @patrick-wilken if the MT recipes make active use of this "feature". If yes, then we should rather have FALSE the default.

@patrick-wilken
Copy link

As said on Slack, I would be okay with having to set TASK_INPUTS_MUST_BE_AVAILABLE to make our current recipes work. WAIT_PERIOD_FOR_INPUTS_AVAILABLE=0 would then also make sense until we stop using path prefixes, right?

Copy link
Contributor

@michelwi michelwi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As said on Slack, I would be okay with having to set TASK_INPUTS_MUST_BE_AVAILABLE to make our current recipes work.

ok.

WAIT_PERIOD_FOR_INPUTS_AVAILABLE=0 would then also make sense until we stop using path prefixes, right?

yes, unless you want to wait unnecessarily for each expected-missing input.

@albertz albertz merged commit d78596a into master Dec 15, 2023
3 checks passed
@albertz albertz deleted the albert-task-wait-inputs-159 branch December 15, 2023 14:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Task.run, wait for inputs
3 participants