Skip to content
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

Websocket: first commit #54

Merged
merged 1 commit into from
Aug 21, 2024

Conversation

rd4com
Copy link

@rd4com rd4com commented Aug 20, 2024

Hello, here is what we've got so far:

  • upgrade http request to websocket
  • send all size
  • receive all size
  • works with select for non-blocking

all sizes means len(data) <=125, <2**16, <((2**64)>>8)

 

Many things not done:

  • ping/pong
  • fragment message

(and more)

 

There is a need to think about how it's gonna be integrated,
we'll need to organize the flags in aliases, for example.

Error handling seem like a great priority 👍

 

Note that the code appears to be more complicated than what it is doing,
because we're doing extra steps to work with ByteArray (PythonObject) in this example.

@saviorand
Copy link
Collaborator

Thanks, looks great! Will review tomorrow. Will let you know if I have any questions

@saviorand saviorand merged commit efa92a9 into Lightbug-HQ:feature/websocket Aug 21, 2024
@saviorand
Copy link
Collaborator

Alright, merged into the branch. Let's open new PRs for improvements/refactors. I've also merged the nightly updates into this, as main is on the latest stable version and breaks with nightly.

The main to-do's I'm seeing so far before merging into main, in order of importance:

  1. Integrate with the rest of the code by adding this websocket infra as an option into the SysServer or making a separate server struct for this. For now thinking what's the best way to approach this for best ergonomics/developer experience
  2. Replace python imports with equivalent Mojo/libc calls. E.g. in socketwe've got implementations interfacing with C via external_calls for operations like opening socket, recv, bind and so on
  3. Localhost IP/port is currently hardcoded, can make it configurable since it's already passed as a parameter, the magic number can be an actual constant (alias). General clean up - we should remove prints/extra comments
  4. The index.html file is useful for trying it out, but we should remove it and write unit tests + a simple e2e test instead

I can work on 1, 2 and 4. Can you look into 3? Also if you have any other scope in mind feel free to open new PRs into the same branch

@saviorand saviorand mentioned this pull request Sep 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants