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

Add/websocket #143

Merged
merged 44 commits into from
Nov 18, 2024
Merged

Add/websocket #143

merged 44 commits into from
Nov 18, 2024

Conversation

AHReccese
Copy link
Member

@AHReccese AHReccese commented Oct 18, 2024

Reference Issues/PRs

#41
In this PR, the ability to use websocket as the communication medium for both client and server side is implemented. but as you know, websocket brings asynchronous context to the board and we do have the ability to let user define for example 3 ml models and push the fit function to the server and wait till get them done. but we need to design a proper mechanism for this purpose. right now it the underlying medium is using websocket and the point is for example that there is one connection instead of connection creation per request to server (like REST protocol).

What does this implement/fix? Explain your changes.

Any other comments?

@AHReccese AHReccese added this to the pymilo v1.1 milestone Oct 18, 2024
@AHReccese AHReccese self-assigned this Oct 18, 2024
@AHReccese
Copy link
Member Author

All checks are passed, but for weird reason, the windows python 3.11 fails, as you can see in the attached picture, all tests are passed but the process gets killed, I will re-run this failed job in another time.

image

tests/test_ml_streaming/run_server.py Outdated Show resolved Hide resolved
pymilo/streaming/communicator.py Outdated Show resolved Hide resolved
@AHReccese
Copy link
Member Author

I will update README.md in a separate PR.
@sepandhaghighi

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@sepandhaghighi sepandhaghighi added the major major changes, to be reviewed in 10 days label Nov 12, 2024
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
dev-requirements.txt Outdated Show resolved Hide resolved
@AHReccese
Copy link
Member Author

@sepandhaghighi
I'm done, I could finally handle both your feedback and also fix the issues. You can review it now.

Copy link
Member

@sepandhaghighi sepandhaghighi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AHReccese
Everything looks good to me for now 💯
But remember that we need to manage the versions of pydantic and scipy in the future. If we cannot control their latest versions in the dev-requirements.txt, we should remove them from PyMilo requirement files.

@AHReccese AHReccese merged commit 8604c64 into dev Nov 18, 2024
24 of 25 checks passed
@AHReccese AHReccese deleted the add/websocket branch November 18, 2024 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
major major changes, to be reviewed in 10 days new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants