-
Notifications
You must be signed in to change notification settings - Fork 21
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
chore: update multiproc to use explicit ports for connection #127
Conversation
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #127 +/- ##
=======================================
Coverage 95.21% 95.21%
=======================================
Files 33 33
Lines 1359 1359
Branches 89 89
=======================================
Hits 1294 1294
Misses 36 36
Partials 29 29 ☔ View full report in Codecov by Sentry. |
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
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.
Same change for sourcetransformer/multiproc_server.py?
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.
Minor comments
@@ -87,8 +85,9 @@ def __init__( | |||
("grpc.so_reuseport", 1), | |||
("grpc.so_reuseaddr", 1), | |||
] | |||
self._sock_path = sock_path | |||
self._process_count = int(os.getenv("NUM_CPU_MULTIPROC") or os.cpu_count()) |
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.
self._process_count = int(os.getenv("NUM_CPU_MULTIPROC") or os.cpu_count()) | |
self._process_count = int(os.getenv("NUM_CPU_MULTIPROC")) or (2 * os.cpu_count()) |
default can be twice the cpu count right?
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.
The default should be os.cpu_count() and the upper limit for the user defined can be 2*cpu_count
Signed-off-by: Sidhant Kohli <[email protected]>
Missed uploading those! Have added in this commit |
Kindly explain what this PR does.
Fixes #111