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

Fix --exit-on-error not working and add --skip-completed-runs #1400

Merged
merged 2 commits into from
Mar 8, 2023

Conversation

yifanmai
Copy link
Collaborator

@yifanmai yifanmai commented Mar 2, 2023

  • --exit-on-error was broken by Merge present.py into run.py #1302
  • --skip-completed-runs is useful for re-running helm-run on the same run_specs.conf if some runs fail, without having to create a new run_specs.conf with only the failing runs.

@yifanmai yifanmai requested review from percyliang and teetone March 2, 2023 17:35
@@ -106,6 +117,12 @@ def run_one(self, run_spec: RunSpec):
run_path: str = os.path.join(self.runs_path, run_spec.name)
ensure_directory_exists(run_path)

if os.path.exists(os.path.join(run_path, "scenario_state.json")):
Copy link
Member

Choose a reason for hiding this comment

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

I don't see where skip_completed_runs is being used. Should we check it here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for catching the bug. Added the flag check.

@yifanmai yifanmai requested a review from teetone March 6, 2023 19:12
@yifanmai
Copy link
Collaborator Author

yifanmai commented Mar 8, 2023

@teetone could you take a look at this?

Copy link
Member

@teetone teetone left a comment

Choose a reason for hiding this comment

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

Looks good!

@yifanmai yifanmai merged commit aa95d76 into main Mar 8, 2023
@yifanmai yifanmai deleted the yifanmai/runner-improvements branch March 8, 2023 20:40
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.

2 participants