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

feat: class based refactor for SDK #129

Merged
merged 78 commits into from
Jan 24, 2024
Merged

Conversation

kohlisid
Copy link
Contributor

@kohlisid kohlisid commented Jan 9, 2024

Fixes #106

  1. Refactor the SDKs to accept both classes and function as handlers
  2. Seperated common server utilities
  3. Abstracted server start call to have common function for all server types, do not need to use aiorun seperately for async functions. Makes it easier to switch between them for the UDF owner.
  4. Different server classes for each UDF
  5. Enable UV loop for async execution

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]>
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]>
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]>
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]>
Signed-off-by: Sidhant Kohli <[email protected]>
Signed-off-by: Sidhant Kohli <[email protected]>
@kohlisid kohlisid marked this pull request as draft January 18, 2024 02:53
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]>
@kohlisid kohlisid requested a review from vigith January 22, 2024 20:01
Signed-off-by: Sidhant Kohli <[email protected]>
Comment on lines +40 to +42
prime_class = PrimeMap()
# Server count is the number of server processes to start
grpc_server = MapMultiprocServer(prime_class, server_count=server_count)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
prime_class = PrimeMap()
# Server count is the number of server processes to start
grpc_server = MapMultiprocServer(prime_class, server_count=server_count)
# Server count is the number of server processes to start
# non-thread safe
grpc_server = MapMultiprocServer(instance=PrimeMap(), server_count=server_count)
# thread safe since we create an instance for every message
grpc_server = MapMultiprocServer(class=PrimeMap, server_count=server_count)

don't implement but just think

Signed-off-by: Sidhant Kohli <[email protected]>
@kohlisid kohlisid requested a review from vigith January 23, 2024 06:43
@kohlisid kohlisid marked this pull request as ready for review January 23, 2024 17:49
Copy link
Member

@ab93 ab93 left a comment

Choose a reason for hiding this comment

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

Some comments!

examples/map/forward_message/example.py Outdated Show resolved Hide resolved
examples/map/forward_message/example.py Outdated Show resolved Hide resolved
examples/reduce/counter/example.py Outdated Show resolved Hide resolved
examples/sideinput/simple-sideinput/example.py Outdated Show resolved Hide resolved
examples/sink/async_log/example.py Outdated Show resolved Hide resolved
pynumaflow/mapper/_dtypes.py Outdated Show resolved Hide resolved
pynumaflow/mapper/async_server.py Show resolved Hide resolved
pynumaflow/mapper/multiproc_server.py Show resolved Hide resolved
pynumaflow/reducer/_dtypes.py Outdated Show resolved Hide resolved
Signed-off-by: Sidhant Kohli <[email protected]>
@kohlisid kohlisid requested a review from ab93 January 23, 2024 20:03
examples/sink/async_log/example.py Outdated Show resolved Hide resolved
Signed-off-by: Sidhant Kohli <[email protected]>
@kohlisid kohlisid requested a review from ab93 January 24, 2024 02:08
examples/reduce/counter/Makefile Outdated Show resolved Hide resolved
pynumaflow/reducer/async_server.py Outdated Show resolved Hide resolved
pynumaflow/reducer/async_server.py Outdated Show resolved Hide resolved
Signed-off-by: Sidhant Kohli <[email protected]>
@kohlisid kohlisid merged commit 39ec103 into numaproj:main Jan 24, 2024
10 checks passed
@kohlisid kohlisid deleted the class-refactor branch February 28, 2024 13:35
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.

Class based approach for passing the handler
3 participants