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

Suspend and Resume #118

Merged
merged 5 commits into from
Jul 7, 2023
Merged

Suspend and Resume #118

merged 5 commits into from
Jul 7, 2023

Conversation

shadeofblue
Copy link
Contributor

No description provided.

@shadeofblue shadeofblue self-assigned this Jul 5, 2023
bump the minor version to 0.3

self._startup_finished = True

async def start(self):
async def start(self, resume=False):
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer start, _create_networks and _start_service with resume=True to be separated methods. e.g. resume, _resume_networks and _resume_service.

This would limit the number of times resume arg is passed around and frequency of resume being checked in if statements.

I also wonder if resume realy is an options of exciting start command or should it be a separated command.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

as you can see there's still considerable overlap in the logic that happens nevertheless when executing those steps, so in my opinion "resume" still seems like a mode of starting the runner rather than a completely separate operation...

otherwise, we'll have to pull the logic to a larger number of internal methods, which will make the code more complicated...

so while I don't right out disagree with you, I feel specifying resume as a flag of the binary and a parameter of the various functions that start the components is reasonable and don't see strong arguments to separate them completely...

@shadeofblue shadeofblue merged commit b1a15ac into main Jul 7, 2023
@shadeofblue shadeofblue deleted the blue/suspend-resume branch July 7, 2023 08:52
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