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.

Sign up for GitHub

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

Jump to bottom

Update scenario status #378

Merged
merged 1 commit into from
Jan 25, 2021
Merged

Update scenario status #378

merged 1 commit into from
Jan 25, 2021

Conversation

rouille
Copy link
Collaborator

@rouille rouille commented Jan 25, 2021

Purpose

Update scenario status before launching a simulation.

What the code is doing

Update the _scenario_status attribute of the Execute.

Where to look

The _update_scenario_status method is invoked in the _check_if_ready method which, in turn, is invoked in the launch_simulation method

Time estimate

2 min

@rouille rouille added the bug Something isn't working label Jan 25, 2021
@rouille rouille added this to the Hey Joe milestone Jan 25, 2021
@danielolsen
Copy link
Contributor

Looks reasonable, I'll test with the next scenario I create and report back.

jenhagg
jenhagg approved these changes Jan 25, 2021
View reviewed changes
Copy link
Collaborator

@jenhagg jenhagg left a comment

Choose a reason for hiding this comment

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

I ended up testing the same fix in #376

BainanXia
BainanXia approved these changes Jan 25, 2021
View reviewed changes
Copy link
Collaborator

@BainanXia BainanXia left a comment

Choose a reason for hiding this comment

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

It works! I tested it with scenario 1995 just now. Without print_scenario_status, we can still launch scenario now.

danielolsen
danielolsen approved these changes Jan 25, 2021
View reviewed changes
Copy link
Contributor

@danielolsen danielolsen left a comment

Choose a reason for hiding this comment

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

Also works with scenario number 1996 :)

@rouille rouille merged commit 12e1dcc into develop Jan 25, 2021
@rouille rouille deleted the ben/status branch January 25, 2021 20:16
@danielolsen danielolsen mentioned this pull request Jan 25, 2021
Closed
@ahurli ahurli mentioned this pull request Mar 11, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@danielolsen danielolsen danielolsen approved these changes

@BainanXia BainanXia BainanXia approved these changes

@jenhagg jenhagg jenhagg approved these changes

@ahurli ahurli Awaiting requested review from ahurli

Assignees

@rouille rouille

@BainanXia BainanXia

@jenhagg jenhagg

@ahurli ahurli

Labels
bug Something isn't working
Projects
None yet
Milestone
Hey Joe
Development

Successfully merging this pull request may close these issues.
5 participants
@rouille @danielolsen @BainanXia @jenhagg @ahurli