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

Change centralized launcher signature #2094

Merged
merged 42 commits into from
Apr 30, 2024
Merged

Change centralized launcher signature #2094

merged 42 commits into from
Apr 30, 2024

Conversation

tokatoka
Copy link
Member

it should accept is_main as boolean

@domenukk
Copy link
Member

domenukk commented Apr 23, 2024

Hmm this way we cannot simply swap out launchers anymore, could this be part of the mgr instead?
Or metadata in State?

@tokatoka
Copy link
Member Author

Hmm this way we cannot simply swap out launchers anymore

IMO run_client should be doing different things depending on if it is the main node or the non-main node.
the main node should not do fuzzing. therefore there is no swappability between launcher and centralized
The user is responsible to do different thing in run_client closure

Most common pattern is if it is main node. users should use empty tuple for the stages.

@domenukk
Copy link
Member

Then centralized should probably have an extra callback, but the normal one/the one that fuzzes should have the same signature to keep them swappable

@s1341
Copy link
Collaborator

s1341 commented Apr 24, 2024

Agree with @domenukk - keep run_client the same and add a run_main or something.

@tokatoka
Copy link
Member Author

Then centralized should probably have an extra callback, but the normal one/the one that fuzzes should have the same signature to keep them swappable

Agree with @domenukk - keep run_client the same and add a run_main or something.

Roger

@tokatoka tokatoka merged commit 2f7c19e into main Apr 30, 2024
100 checks passed
@tokatoka tokatoka deleted the centralized_sig branch April 30, 2024 17:44
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.

4 participants