-
Notifications
You must be signed in to change notification settings - Fork 3
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
Dn reliable websockets #7 #8
Conversation
* Un | ||
* s to all current subscriptions by closing the WebSocket to the Vantiq |
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.
accidental typo here, I think.
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.
LGTM, just a few minor nits, but once those are addressed this is good to go!
} | ||
|
||
/**Acknowledge the receipt of a reliable message*/ |
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.
to be consistent with the other method comments this should be multiline:
/**
Acknowledge the receipt of a reliable message
*/
this.accessToken = accessToken; | ||
} | ||
} | ||
public void subscribe(String path, SubscriptionCallback callback, Map<String, Object> parameters) { |
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.
newline between the end of the class def and function def please
@BeforeClass | ||
public static void setUpIntgTest() throws Exception { | ||
// Pull values from java properties, so the credentials are not checked in | ||
server = System.getProperty("server"); | ||
username = System.getProperty("username"); | ||
password = System.getProperty("password"); | ||
|
||
if(server == null || username == null || password == null) { |
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.
Leave this check in, just extend it to also check the token
vantiq.authenticate(username, password, handler); | ||
if (!username.equals("") && !password.equals("")) { | ||
vantiq.authenticate(username, password); | ||
} else { |
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 doesn't account for if the token wasn't specified
Add support for acknowledging events for reliable resources and create persistent/reconnectable subscriptions