-
Notifications
You must be signed in to change notification settings - Fork 269
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
POC: First working implementation of websockets. #526
Conversation
WebSocketHandler.open(self, *args, **kwargs) | ||
|
||
def on_message(self, message): | ||
print(u"Received: " + 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.
I know i know, just for debugging currently
@pymedusa/developers As we've decided that removing the messages requests is high prio, and scheduled for the 0.1.1 milestone, i'd like everyone to start looking at the implementation, and give feedback. Any is welcome. |
33de253
to
4ef9d57
Compare
212ab3a
to
53888cd
Compare
@p0psicles are you able to fix this PR to have only the WS stuff and remove all the extra apiv2 commit, etc. |
Yeah will be working on this next |
0c9b23f
to
85413f4
Compare
static/js/ajax-notifications.js
Outdated
var msg; | ||
try { | ||
msg = JSON.parse(evt.data); | ||
} catch(e) { | ||
} catch (err) { |
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.
@OmgImAlexis shouldn't we log.error(err);
here?
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.
No and this isn't err
this is e
. err should return a new error object where as this is an exception object. There's not actually any errors as this is used to check if the string if a json encoded string or just a normal string.
If xo is giving an error just add this to the right of it.
// eslint-disable-line-only ADD_ERROR_NAME_HERE
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.
@fernandog Can you revert this line's change?
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.
Sure
@pymedusa/developers @pymedusa/administrators @pymedusa/contributors Can you guys please try this so it can be merged as it's been open for almost a year now. |
static/js/ajax-notifications.js:31:28 ✖ 31:28 Unexpected space before function parentheses. space-before-function-paren ✖ 35:11 Expected space(s) after "catch". keyword-spacing ✖ 35:17 The catch parameter should be named err. unicorn/catch-error-name ✖ 49:5 A function with a name starting with an uppercase letter should only be used as a constructor
56b6fe4
to
4c6f834
Compare
This will show the user a pNotify with an informative message to check wiki.
* Removed evt as we don't need it.
@p0psicles can we please merge this into dev soon? |
This is a working POC, for replacing the current REST polling mechanisme we use for the Pnotifications, with Websockets. For more info please refer to FR: #495.
Currently it's just a POC and based on #432 , for discussing the actual implementation.
Replaced the Rest call used for the notification, with the websockets.