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

Use tcp manager when "tcp_manager" feature is on #1675

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

s1341
Copy link
Collaborator

@s1341 s1341 commented Nov 19, 2023

No description provided.

@s1341 s1341 changed the title Use tcp manager when "tcp_manager" \feature is on Use tcp manager when "tcp_manager" \eature is on Nov 19, 2023
@s1341 s1341 changed the title Use tcp manager when "tcp_manager" \eature is on Use tcp manager when "tcp_manager" feature is on Nov 19, 2023
@andreafioraldi
Copy link
Member

I don't get this. LLMP TCP etc can coexist, we don't need a new feature

@andreafioraldi
Copy link
Member

I think the way to go here to avoid code reuse is a macro that define Launcher with different names (CentralizedLauncher, TCPLauncher etc) changing the trait bound to the different kind of managers

@s1341
Copy link
Collaborator Author

s1341 commented Nov 20, 2023

Are you sure?

@domenukk
Copy link
Member

Yes sounds good. We might also have other launchers for other managers in the future

@domenukk
Copy link
Member

domenukk commented Jan 1, 2024

As long as we don't have a better idea, maybe we want to merge this anyway? It's useful to have a tcpmanager enabled launcher sometimes..

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.

None yet

3 participants