@xviz/server module #453
@xviz/server module #453
Conversation
twojtasz
force-pushed
the
xviz-server-module
branch
from
e739c98
to
11a3963
Compare
Replace our adhoc server with a structured middleware based implementation. This server is setup to read from XVIZ Data Source to support any source such as files or a runtime conversion from other formats.
We cannot send 'object' down a websocket. The prior code was too complicate and this is a simplification. This will be codified in a test in a follow-up diff.
- Added test classes for server testing - Broke out XVIZ message to middleware handling into a class
twojtasz
force-pushed
the
xviz-server-module
branch
from
11a3963
to
6657c2e
Compare
There was a problem hiding this comment.
These docs here are really great and complete! I have given read through the docs but need to do another pass on the code itself while referencing the docs to make sure I understand all the concepts. Can I want to have a glossary somewhere with all the main bits like:
- Provider
- Handler
- Session
- Middleware
- ...
So the two overview images of IO and server are the best we have right now, but something really concise to jog my memory while reviewing the code would help.
There was a problem hiding this comment.
This code is super SOLID @twojtasz it took two monitors and a pinned octotree to review so much of it at once though.
The comment I have on the testing is that we might look at adding testing at lower levels eventually because right now we just have it at the server level, and we don't have any flow types on this either so it would be easy for bugs to creep in at that level (depending on how many paths the server level tests cover).
| this.options = options; | ||
| } | ||
|
|
||
| writeSync(name, data) { |
There was a problem hiding this comment.
The unused name parameter in this field is confusing.
There was a problem hiding this comment.
I can move the name to the send argument, that way it can be ignored by JS impl's that don't require it.
There was a problem hiding this comment.
So this runs a bit deeper than I thought. The writeSync i related to the other write messages (specifically writeMetadata and writeFrame). Since we are changing the writeFrame => writeMessage I"ll make this related IO change in that PR
| } | ||
| } | ||
|
|
||
| onTransformLogDone(msg) { |
There was a problem hiding this comment.
Yeah it is weird that some of the message we send back and forth as GLB but some we always send as JSON.
There was a problem hiding this comment.
I should just do the same I do for onStateUpdate(msg) here. I'm cheating because this is a server generated msg but yeah, this is not robust.
There was a problem hiding this comment.
curiously, in order to support the uniform handling of message I need the above writeMessage change :) The reason being is that this code is taking short cut by assuming it knows the data format and writing directly to the socket. The more uniform approach delegates the formatting issue to the writer. But the current IO interface has write functions only for Metadata and Frame. So this too will be in #454 which is turning out to be a very much needed correction on the API
…e" nature with underscore prefix
Yeah, definitely need to avoid the large commit and break out design way earlier. Sadly a lot of it was "discovered" only once I was working through the original server and never stopped to formalize the whole thing like I should have. Thanks for slogging through it all! |
Proper module to replace our adhoc server script with a structured middleware-based implementation. Key documents are: - RFC ./dev-docs/004-server-module-rfc.md - User Docs ./docs/api-reference/server - Module Readme ./modules/server/README.md
Proper module to replace our adhoc server script with a structured middleware-based implementation. Key documents are: - RFC ./dev-docs/004-server-module-rfc.md - User Docs ./docs/api-reference/server - Module Readme ./modules/server/README.md
Proper module to replace our adhoc server script with a structured middleware-based implementation. Key documents are: - RFC ./dev-docs/004-server-module-rfc.md - User Docs ./docs/api-reference/server - Module Readme ./modules/server/README.md
Provide a full featured XVIZ server module that can full support the XVIZ protocol and allow customization