numaproj / numaflow-python

Numaflow Python SDK
Apache License 2.0
50 stars 17 forks source link

Sourcetransform example raises syntax error `Argument missing for parameter "source_transform_instance"` #167

Closed th0ger closed 3 months ago

th0ger commented 3 months ago

Hi, I'm writing a custom source transformer.

With pynumaflow 0.6.0, I tried to use

from pynumaflow.sourcetransformer import SourceTransformer
from .handler import handler
grpc_server = SourceTransformer(handler=handler)
grpc_server.start()

The syntax was okay, the transformer container started, but numa container kept raising: 2024/05/22 13:27:43 Server info file /var/run/numaflow/sourcetransformer-server-info is not ready.

I saw a recent serverInfoPath fix https://github.com/numaproj/numaflow/issues/1456 that could be related. Therefore tried to upgrade to pynumaflow 0.7.0.

Apparently the API has changed in 0.7.0 since we now have to use SourceTransformServer (example not specifying API version?):

from pynumaflow.sourcetransformer import SourceTransformServer
from .handler import handler
grpc_server = SourceTransformServer(handler=handler)
grpc_server.start()

But for SourceTransformServer I get a syntax error Argument missing for parameter "source_transform_instance", even though I follow the latest event_time_filter/example.py

What's the right syntax?

In general, it's fair enough that you introduce breaking changes below <1.0.0, but I'ts really hard to figure out where they are and how to resolve them. I would ask for more general API documentation and better release notes on API changes so it's easier to upgrade. Also could you somehow pin the examples to exact pynumaflow version (or the range of allowed versions) so we know where they apply?

Thanks in advance!


Message from the maintainers:

Impacted by this bug? Give it a 👍. We often sort issues this way to know what to prioritize.

vigith commented 3 months ago

are you facing the same problem with 0.7? We have some breaking changes, could you please try 0.7?

th0ger commented 3 months ago

Yes I upgraded to pynumaflow 0.7.0. (numaflow is the latest release.)

  1. I figured that SourceTransformServer() takes arg source_transform_instance instead of handler, but it's still accepts the handler. So it works now.

  2. I get Server info file /var/run/numaflow/sourcetransformer-server-info is not ready for both 0.6.0 and 0.7.0 but maybe I should open another issue for this?

vigith commented 3 months ago

~Are you using Numaflow 1.2.1? If yes, it should be working. Please open an issue.~

vigith commented 3 months ago

nm.. i see that you are using the latest, sorry about that. is the example code working for you? if no, please open an issue.

th0ger commented 3 months ago

Today I no longer get Server info file /var/run/numaflow/sourcetransformer-server-info is not ready so can't reproduce.

vigith commented 3 months ago

I get Server info file /var/run/numaflow/sourcetransformer-server-info is not ready for both 0.6.0 and 0.7.0 but maybe I should open another issue for this?

Please keep in mind that if your __init__ takes a long time, then the server startup will be delayed, and that will materialize as the server-info file is missing. This can also trigger container restarts if it takes more than the readiness probe timeout.

th0ger commented 3 months ago

I got more Server info file /var/run/numaflow/*-server-info is not ready errors today for other pipelines, fixed by upgrading pynumaflow 0.6.0 -> 0.7.0,

kohlisid commented 3 months ago

@th0ger That seems to be consistent with the changes as in 0.7.0 we introduced different server info files for each of the udf types hence this message. The numa would have been on latest version and expecting the seperate info files from server(SDK).