-
Notifications
You must be signed in to change notification settings - Fork 230
NATS SDK compatibility #572
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
Conversation
incoming NATS connections will use this user
…n in UI)" This reverts commit bef727f.
modify GetMessages and GetMessageDetails to gracefully allow non-memphis-header messages
| // NATS SDK, means we extract username from the token field | ||
| splittedToken := strings.Split(client.opts.Token, connectItemSep) | ||
| if len(splittedToken) != 2 { | ||
| client.Warnf("handleConnectMessage: missing username or connectionId") |
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.
connection id -> token
| splittedToken := strings.Split(client.opts.Token, connectItemSep) | ||
| if len(splittedToken) != 2 { | ||
| client.Warnf("handleConnectMessage: missing username or connectionId") | ||
| return errors.New("missing username or connectionId") |
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.
same here
| } | ||
| username := splittedToken[0] | ||
| client.memphisInfo = memphisClientInfo{username: username, isNative: false} | ||
| return nil |
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 function should validate the user validations part + send proper analytics (new event user-connect-nats-sdk)
| func (js *jetStream) apiDispatch(sub *subscription, c *client, acc *Account, subject, reply string, rmsg []byte) { | ||
| // No lock needed, those are immutable. | ||
| s, rr := js.srv, js.apiSubs.Match(subject) | ||
| wrappedSubject := memphisFindJSAPIWrapperSubject(c, subject) |
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 need you to explain it to me
server/memphis_helper.go
Outdated
|
|
||
| msgs, err := s.memphisGetMsgs(stationName.Intern()+".final", | ||
| stationName.Intern(), | ||
| msgs, err := s.memphisGetMsgs(stationName.Intern(), |
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.
Why have you deleted it?
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.
Apparently, there was no use of that parameter inside the function, just cleaned it.
| @@ -1,7 +1,7 @@ | |||
| { | |||
| "files": { | |||
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.
remove all static files from the PR
…or non-native sdk clients
83c232d to
762585d
Compare
server/memphis_handlers_stations.go
Outdated
| removeFunc := nonNativeRemoveStreamFunc | ||
| if removeFunc == nil { | ||
| if !station.IsNative { | ||
| s.Fatalf("non native station uses native remove stream function") |
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.
please change to error and return the user an error
| } | ||
| analyticsParams := []analytics.EventParam{param} | ||
| analytics.SendEventWithParams(user.Username, analyticsParams, "user-connect-sdk") | ||
| analytics.SendEventWithParams(username, analyticsParams, "user-connect-sdk") |
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.
please send new event user-connect-nats-sdk when this is non native connection
plain error on nats-sdk usage in a native memphis station
No description provided.