WIP add websocket support #64
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
=======================================
Coverage 90.56% 90.56%
=======================================
Files 5 5
Lines 53 53
=======================================
Hits 48 48
Misses 5 5
Continue to review full report at Codecov.
|
|
Would be happy to get some help as I'm a bit stuck though. On the testcase of #63 I get: |
|
Looks like this handler function should take two args, as it did originally? |
hustf
reviewed
Jul 26, 2018
There was a problem hiding this comment.
Sorry for the delayed upgrade of Websockets. I hope to get time for that next week. If you choose to use Websockets, I recommend readprotected and writeprotected. It reduces your error handling and makes error messages more readable. I believe this is already working on the branch for 0.7.
|
I've changed the handler back to how it was but it doesn't seem to fix the problem, I get exactly the same error. I feel that EDIT: actually it's a different problem, @hustf thanks for the tip. Where exactly should I switch to |
|
Pietro, that was just general advice. The move to HTTP increased the number
of different error messages occuring occasionaly, and in my own
"production" code the error handling code grew out of proportion to the
simple task of reading and writing. Hence the added interface protectedread
and write, and the inclusion of benchmarks to evaluate the disadvantage
related to error handling. I am not on a computer this week and dont have
the opportunity to study this code well enough to give precise advice.
Looking forward to finding out how this package works later. Good luck!
Den tor. 26. jul. 2018, 18:35 skrev Pietro Vertechi <
[email protected]>:
… I've changed the handler back to how it was but it doesn't seem to fix the
problem, I get exactly the same error. I feel that wdefault doesn't work
properly in the new framework but I'm not sure why.
@hustf <https://github.com/hustf> thanks for the tip. Where exactly
should I switch to readguarded ? It seems that only the echo function
uses read or write but it's not used in the code that's being tested here.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#64 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ANTatKlUOFfXxzKyYjxV9BEk22b56p6Uks5uKe-6gaJpZM4VZq6x>
.
|
minor cleanup
|
Sorry for the missing follow up from my side. Time ran out, and next week is juliacon... |
|
Any updates on this? Or anyone want to hack on it while we're at JuliaCon 🙂 ? |
|
I'm still stuck... Maybe we could try to fix this during the Hackathon? |
|
Count me in |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
An attempt to fix #63. I don't actually have examples of this working as it relies on WebSockets also updating to Julia 0.7. At the very least the websocket method of
servedoesn't give method error anymore.