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

Finishing clusterd #1165

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

Finishing clusterd #1165

wants to merge 4 commits into from

Conversation

akbashev
Copy link
Contributor

Quite often people ask "How actors find each other" and clusterd could be a good example to show discoverability.

Again just stealing your code here @ktoso 🙂 Will just try to finish and prettify.

@akbashev
Copy link
Contributor Author

@ktoso we can focus on your PR though, can close this one. It's just easier for me to finish it here.

@ktoso
Copy link
Member

ktoso commented Oct 16, 2024

Happy for you to take it over. Fixed up CI so we now just do 5.10+ jobs.

@ktoso ktoso closed this Oct 16, 2024
@ktoso ktoso reopened this Oct 16, 2024
# Conflicts:
#	Sources/MultiNodeTestKitRunner/boot+MultiNodeTestKitRunner+Exec.swift
@akbashev akbashev marked this pull request as ready for review November 5, 2024 15:33
Comment on lines +494 to +496
case `static`(endpoints: Set<Cluster.Endpoint>, subscribe: (@escaping (Result<[Cluster.Endpoint], Error>) -> Void, @escaping (CompletionReason) -> Void) -> CancellationToken?)
case dynamic(serviceDiscovery: AnyServiceDiscovery, subscribe: (@escaping (Result<[Cluster.Endpoint], Error>) -> Void, @escaping (CompletionReason) -> Void) -> CancellationToken?)
case clusterDaemon(endpoint: Cluster.Endpoint, initialize: (ClusterSystem) -> Void)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Decided to put subscribe and initialize closures here, as otherwise it's quite confusing to have optional values in settings itself.

@akbashev
Copy link
Contributor Author

akbashev commented Nov 5, 2024

@ktoso I've went through code, cleaned up some stuff, executable is now called swift-clusterd and tested myself, test is also working.
Except for ClusterD should be a "HIDDEN" member in the membership mentioned in original PR think it's already in a good shape to be merged.

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