-
Notifications
You must be signed in to change notification settings - Fork 2
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
Event stream client #1
Conversation
I added a |
client.go
Outdated
// SubscribeEvents subscribes to events from the default (localhost:20202) | ||
// LiteFS node. | ||
func SubscribeEvents() EventSubscription { | ||
return DefaultClient.SubscribeEvents() | ||
} | ||
|
||
// MonitorPrimary monitors the primary status of the LiteFS cluster via the | ||
// default (localhost:20202) LiteFS node's event stream. | ||
func MonitorPrimary() PrimaryMonitor { | ||
return DefaultClient.MonitorPrimary() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these add much value since they're one-liners. I think you can just remove them.
// DefaultClient is a client for communicating with the default | ||
// (localhost:20202) LiteFS node. | ||
var DefaultClient = &Client{ | ||
URL: "http://localhost:20202", | ||
HTTP: http.DefaultClient, | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works. You could also have NewClient()
return a client with defaults set.
// EventSubscription monitors a LiteFS node for published events. | ||
type EventSubscription interface { | ||
// Next attempts to read the next event from the LiteFS node. An error is | ||
// returned if the request fails. Calling `Next()` again after an error will | ||
// initiate a new HTTP request. ErrClosed is returned if the EventSubscription | ||
// is closed while this method is blocking. | ||
Next() (*Event, error) | ||
|
||
// Close aborts any in-progress requests to the LiteFS node. | ||
Close() | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it was better just having the regular struct. I'm not a huge fan of interfaces unless you really need them. I don't see users mocking this out and if they do, it's pretty easy to mock out the actual HTTP endpoint.
@btoews Sorry for the slow responses. I've been slammed lately. Just some minor feedback and then we can merge it in. 👍 |
This PR adds client APIs for working with the event streams added in superfly/litefs#401. A lower level
EventSubscription
type is added for subscribing to raw events. More generally useful is thePrimaryMonitor
type that continuously monitors the event stream for changes in the cluster primary, allowing current information about the primary to be fetched immediately.Here's a preview of the godocs.