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

python functions cannot find python3 interpreter #5518

Closed
candlerb opened this issue Oct 31, 2019 · 17 comments · Fixed by #5536
Closed

python functions cannot find python3 interpreter #5518

candlerb opened this issue Oct 31, 2019 · 17 comments · Fixed by #5536
Labels
lifecycle/stale type/bug The PR fixed a bug or issue reported a bug

Comments

@candlerb
Copy link
Contributor

candlerb commented Oct 31, 2019

Describe the bug
python functions do not start if the system has "python3" but not "python" (which is the case for modern Ubuntu systems - "python" is python2 and is not installed by default)

To Reproduce
Write a simple function func1.py

from pulsar import Function

class FirstFunction(Function):
    def process(self, item, context):
        log = context.get_logger()
        log.info("Got %r with properties %r" % (item, context.get_message_properties()))

Deploy it:

apache-pulsar-2.4.1/bin/pulsar-admin functions create --name womble --inputs my-topic \
  --py /home/ubuntu/func1.py --classname FirstFunction --log-topic pulsar-log

(side note: have to give absolute path to function, pulsar-admin changes the working directory)

Check its status:

$ apache-pulsar-2.4.1/bin/pulsar-admin functions status --name womble
{
  "numInstances" : 1,
  "numRunning" : 0,
  "instances" : [ {
    "instanceId" : 0,
    "status" : {
      "running" : false,
      "error" : "Cannot run program \"python\": error=2, No such file or directory",
      "numRestarts" : 16,
      "numReceived" : 0,
      "numSuccessfullyProcessed" : 0,
      "numUserExceptions" : 0,
      "latestUserExceptions" : [ ],
      "numSystemExceptions" : 0,
      "latestSystemExceptions" : [ ],
      "averageLatency" : 0.0,
      "lastInvocationTime" : 0,
      "workerId" : "c-standalone-fw-ldex-pulsar.int.example.net-8080"
    }
  } ]
}

Note error: Cannot run program "python": error=2, No such file or directory

Expected behavior
I would expect the function to deploy.

Screenshots
N/A

Desktop (please complete the following information):
N/A

Additional context
This may be the same problem as #5369 - however in that case, it's the shebang line in pulsar's own python code, whereas this one is my own function which is not starting.

I cannot find it stated in Pulsar documentation whether Pulsar supports python2, python3 or both. However python2 is very shortly going to be end-of-life.

Workarounds

Install python2 and write my code in python2. I really don't want to do that, with python2 being end-of-life in 2 months' time.

Symlink python to python3. However in Ubuntu systems, symlinking python to python3 has the potential to cause all sorts of problems. Many applications and scripts depend on python meaning python2 (in which case they will depend on the python-XXX packages rather than python3-XXX). More content here.

Ideally pulsar would try python3 and fallback to python. If pulsar were to be hard-coded to run "python3" instead of "python" that would also be fine IMO, given the impending EOL of python2.

@candlerb candlerb added the type/bug The PR fixed a bug or issue reported a bug label Oct 31, 2019
@candlerb
Copy link
Contributor Author

Hmm, I found docker/pulsar/scripts/set_python_version.sh which suggests that people are making python a symlink to python3

sudo update-alternatives --install /usr/bin/python python /usr/bin/python3 10

As I say, this is generally frowned upon.

I found there is pythonInstanceLocation in WorkerConfig.java. The documentation says "change the python instance location only when you put the python instance jar in a different location" - but I'm looking for the python interpreter, not a jar file. (I don't think it's using anything

I haven't yet found the point where pulsar execs the python interpreter.

@candlerb
Copy link
Contributor Author

candlerb commented Oct 31, 2019

Part of the answer comes from pulsar's debug output: it is running python .../python_instance_main.py ...args...

12:25:45.004 [function-timer-thread-94-1] INFO  org.apache.pulsar.functions.runtime.ProcessRuntime - ProcessBuilder starting the process with args
  python /home/ubuntu/apache-pulsar-2.4.1/instances/python-instance/python_instance_main.py
  --py /tmp/pulsar_functions/public/default/womble/0/func1.py
  --logging_directory /home/ubuntu/apache-pulsar-2.4.1/logs/functions --logging_file womble
  --logging_config_file /home/ubuntu/apache-pulsar-2.4.1/conf/functions-logging/logging_config.ini
  --instance_id 0 --function_id 67fb9828-56ab-49ff-bbbc-56db9510d26c
  --function_version 4f01d696-28da-40b7-b9f5-2283fbe9bd7c
  --function_details '{"tenant":"public","namespace":"default","name":"womble","className":"FirstFunction","logTopic":"pulsar-log","runtime":"PYTHON","autoAck":true,"parallelism":1,"source":{"inputSpecs":{"my-topic":{}},"cleanupSubscription":true},"sink":{},"resources":{"cpu":1.0,"ram":"1073741824","disk":"10737418240"},"componentType":"FUNCTION"}'
  --pulsar_serviceurl pulsar://ldex-pulsar.int.example.net:6650 --max_buffered_tuples 1024
  --port 38423 --metrics_port 36591 --state_storage_serviceurl bk://127.0.0.1:4181
  --expected_healthcheck_interval 30 --secrets_provider secretsprovider.ClearTextSecretsProvider
  --cluster_name standalone

So a configuration option to change "python" to "python3" here would be great, and solve the problem.

@aahmed-se
Copy link
Contributor

the question would be what is the default option if any.

@candlerb
Copy link
Contributor Author

the question would be what is the default option if any.

I would be fine with the default remaining at "python" for backwards-compatibility reasons, as long as there's a way to override it.

@Jennifer88huang-zz
Copy link
Contributor

@wolfstudy could you please help look into the issue? Thank you.

@candlerb
Copy link
Contributor Author

candlerb commented Nov 1, 2019

How about adding a setting to broker.conf / standalone.conf? e.g.

python_interpreter=/usr/bin/python

However I notice at least one place where searching in $PATH is currently used:

./pulsar-client-cpp/docker/Dockerfile:ENV PATH="/opt/python/${PYTHON_SPEC}/bin:${PATH}"

So I suggest:

  1. Default is python_interpreter=python
  2. If the setting starts with slash, it's an absolute path (avoids some edge-case security issues)
  3. Otherwise search for it in $PATH as today

If you don't want a conf setting, then an environment variable would work for me too (PYTHON=/usr/bin/python3)

@aahmed-se
Copy link
Contributor

aahmed-se commented Nov 1, 2019

conf file is a better option, makes it more explicit.

@candlerb
Copy link
Contributor Author

candlerb commented Nov 5, 2019

Please can this be re-opened? That might have been my mistake, but #5536 doesn't fix this issue, it only documents the (bad) workaround.

IMO Pulsar still needs a conf option to set the python interpreter path.

@Jennifer88huang-zz
Copy link
Contributor

Jennifer88huang-zz commented Nov 5, 2019

You can reopen it manually.
Since we relate PR-#5536 to this issue, when that PR is merged, this issue is closed automatically.

@Jennifer88huang-zz
Copy link
Contributor

When you add the conf info in a PR, you can relate the PR to this issue as well.

@sbourkeostk
Copy link
Contributor

I have a potential fix for this. I mentioned it over on https://github.com/streamnative/pulsar/issues/85 but I just noticed now that that's a fork. So here's the info:

It's over here:
sbourkeostk/pulsar@60f5163
The main idea is that it adds a boolean python3 option to the function config.
This involves a change to the function config protobuf definition. Is that ok? If that's not a show stopper, I can bring it up to date and look into getting it to pass the test suite.

@devinbost
Copy link
Contributor

@sijie I've been talking with @sbourkeostk about the change.
We're wondering what the best approach would be regarding adding support for Python3.
I'm wondering if it makes sense to treat Python3 like a different runtime. Perhaps we could change the Python runtime to Python2 and then add Python3 as a new runtime.
(In the picture below from @sbourkeostk 's commit, I'm thinking we would move python3 to PYTHON3 as part of the Runtime Enum.)
image

Thoughts?

@aahmed-se
Copy link
Contributor

@merlimat @jerrypeng @srkukarni Please add your comments. I am don't think this a clean way to add support for switching python interpretes.

@devinbost
Copy link
Contributor

I'm open to better ideas about how to switch Python interpreters.
It's true that making a change in the gRPC layer would touch a lot of things.

@srkukarni
Copy link
Contributor

We already have the runtimeflags that was used to pass any runtime specific flags. We could use the same approach in creating a new flag(say runtimebinary or something like that) that would just take in a string and use that as exec point.

@sbourkeostk
Copy link
Contributor

@srkukarni, you got me thinking. How about reusing customRuntimeOptions? Eg: 3a2b312.
Then setting custom-runtime-options to {"PythonInterpreter": "python3"} or even {"PythonInterpreter": "/opt/python-3.6.5/bin/python3"} would set the interpreter. The reason for using json is to maintain compatibility with KubernetesManifestCustomizer which I think is the only other use of the parameter.

By the way, the protobuf python definitions also need to be recompiled for this to work. Probably should be done in any case. The one in master seems to be old and doesn't include customRuntimeOptions (added with 320cebe)
(The python definitions don't get built by maven - they are been generated manually and tracked by git).

@tisonkun
Copy link
Member

#15376 (PIP-155) should drop support for Python 2 now.

Please open a new issue with repro if it's still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/stale type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants