numaflow-python
numaflow-python copied to clipboard
Streamline API naming for `Server*` / `*Callable`
I just tried to get an overview of the server classes fingerprints (pynumaflow 0.7.0);
| module | class_name | arg_name | arg_type |
|---|---|---|---|
| pynumaflow.mapper | MapServer | mapper_instance | MapSyncCallable |
| pynumaflow.mapper | MapAsyncServer | mapper_instance | MapAsyncCallable |
| pynumaflow.mapper | MapMultiprocServer | mapper_instance | MapSyncCallable |
| pynumaflow.reducer | ReduceAsyncServer | reducer_handler | ReduceCallable |
| pynumaflow.mapstreamer | MapStreamAsyncServer | map_stream_instance | MapStreamCallable |
| pynumaflow.sourcetransformer | SourceTransformServer | source_transform_instance | SourceTransformCallable |
| pynumaflow.sourcetransformer | SourceTransformMultiProcServer | source_transform_instance | SourceTransformCallable |
| pynumaflow.sourcer | SourceServer | sourcer_instance | SourceCallable |
| pynumaflow.sourcer | SourceAsyncServer | sourcer_instance | SourceCallable |
| pynumaflow.sinker | SinkServer | sinker_instance | SyncSinkCallable |
| pynumaflow.sinker | SinkAsyncServer | sinker_instance | AsyncSinkCallable |
| pynumaflow.sideinput | SideInputServer | side_input_instance | RetrieverCallable |
reducer_handlerstands out, suggesting renamereducer_instance.- Consider dropping
Syncsyntax altogether, since it is verbose and will lead to inconsistent syntax over time. For exampleReduceCallableneeds to be renamed toReduceSyncCallablewhenReduceAsyncCallableis implemented. TheServer*class names follows this convention, but the*Callables don't. AsyncSinkCallablestands out toMapSyncCallable/MapAsyncCallable/MapSyncCallablein verb ordering, suggesting consitent namingSinkAsyncCallable.
Message from the maintainers:
If you wish to see this enhancement implemented please add a 👍 reaction to this issue! We often sort issues this way to know what to prioritize.
@th0ger Thanks for pointing this out!
- For the
reducer_handlerstands out, suggesting rename reducer_instance --> This we can change for sure - Consider dropping Sync syntax altogether, ---> For this we might be considering this in the next iteration during refactoring, I want to make the type checking stricter for servers which support multiple types (sync, async, etc) so that we have better handling of inconsistencies
- AsyncSinkCallable stands out to MapSyncCallable Agreed, will change this
You're welcome, thanks for considering this. These are of course just nice-to-have breaking changes that you should wait and bundle with other API changes you plan.