bendudson / py4cl

Call python from Common Lisp
Other
234 stars 31 forks source link

Error not signaled when cannot run Python process #25

Open szmer opened 5 years ago

szmer commented 5 years ago

I'm on Fedora 30 with SBCL 1.4.14-2.fc30. I have python3, but no python (Python 2). Initially I couldn't import any modules because of cryptic end of file on #<SB-SYS:FD-STREAM for "descriptor 23" {1003824B03}> [Condition of type END-OF-FILE] errors, but by digging through the code I discovered I'm supposed to setf py4cl:*python-command* to python3. Now it works great.

I also see that python-start-if-not-alive should've signalled a better error in my case. I think the problem is because the (python-alive-p) check happens when bash, or whatever, is still busy failing. This works:

(defun python-start-if-not-alive ()
    "If no python process is running, tries to start it.
  If still not alive, raises a condition."
    (unless (python-alive-p)
      (python-start))
    (sleep 1) ; give the process some time to fail
    (unless (python-alive-p)
      (error "Could not start python process")))

Sadly I don't know how to fix it in a sane way.

I also think the error here could suggest checking your py4cl:*python-command* for a probable cause.

bendudson commented 5 years ago

Thanks! Yes, the error message could definitely be better. I guess it's also possible that the python process may crash or hang at some later time so the library should handle that better. Perhaps just handling end-of-file conditions in a better way, with a more useful error message?

szmer commented 5 years ago

Yes, now I see that these CL standard end-of-file conditions can happen when writing to the stream from various places in callpython.lisp. Handling them would probably replace (python-alive-p) checks. I can try my hand in writing a pull request if you want, but I understand that it would change many places in code, and these cases probably should be tested.

As to the error message, I imagine it would suggest checking configuration (like that global variable) if you get the error immediately, and ??? otherwise. Many things can mess up a process.

(Edit: and maybe do it as a condition rethrow to let the client handle it in some custom way).

One thing I was also thinking of is giving a "debug" option to print the Python/child process output directly in Lisp REPL. (Potentially it could also alert people that they use different version of Python than they thought etc.). Don't know if it would make sense with the method of communicating with the process that you are using.

digikar99 commented 5 years ago

One way to better the error could be to wrap the body of dispatch-messages in a handler-case (or restart-case?) and signal an error with a better name (say python-process-end.

Also, a simple way to reproduce the error (so as to be able to write test code) could be (python-eval "exit()") or (python-exec "raise SystemExit").

One thing I was also thinking of is giving a "debug" option to print the Python/child process output directly in Lisp REPL.

I am not sure I quite get that - are you referring to the following code producing output after it completes - or something else:

(python-exec "
for i in range(10):
  import time
  time.sleep(1)
  print(i)")

(Potentially it could also alert people that they use different version of Python than they thought etc.).

May be, we can print the (python-version-info) (and *python-command*?) every time on package load.

szmer commented 5 years ago

Yes, I would use handler-case and raise a new condition, because I don't think there's a meaningful way to restart and carry on here (?). I was thinking that the users may then handle this new condition, try to spawn a new Python process, and then rerun whatever code they were trying to run.

As to the test case, ending process from inside Python (as you propose) is probably the same as failing to run Python at all, although there could be some edge cases. I don't know. My thinking was to try to run some bogus *python-command* (like nohtypnohtyp) that would give us an EOF error, but then someone may have a real program named like this.

I am not sure I quite get that - are you referring to the following code producing output after it completes

Yes, to see Python prints as Lisp prints (although doesn't this work by default?). I thought that people could see "python: No such file or directory" from Bash when they don't have python, but probably handling the EOF error is a better solution.

May be, we can print the (python-version-info) (and python-command?) every time on package load.

It would be nice, at least as an option that one can turn off. (I'm assuming you meant "when starting the Python process", because from my understanding loading the package alone doesn't start it)

digikar99 commented 5 years ago

Yes, to see Python prints as Lisp prints (although doesn't this work by default?).

One possible way is implemented using multithreading in py4cl2 - however, this breaks code if someone has used something like - (with-output-to-stream (*standard-output*) ...) - and therefore, may be, that method shouldn't be ported here.

[However, it seems like you mean something else when you mention the next things in reference to (python-version-info).]

Yup, the python process doesn't start wen package is loaded - though it would be trivial (and non-breaking?) to make it start during package load.

szmer commented 5 years ago

I think the current state (deferring the start of the python process) is better in that regard, for example allowing for setting some options before it is started.

As to "python: No such file or directory", I mean just what would happen in the terminal if you would enter python command on a system where it's not installed. This obviously isn't printed from Python itself. This whole idea with redirecting output is an aside and as I said, probably just an appropriate error on process EOF deals with this problem of diagnostics in the best way.

digikar99 commented 5 years ago

One possible way is implemented using multithreading in py4cl2 - however, this breaks code if someone has used something like - (with-output-to-stream (*standard-output*) ...) - and therefore, may be, that method shouldn't be ported here.

I, finally, have it implemented (reliably) using macros and thread synchronization in the form of (with-python-output &body body) macro!

digikar99 commented 4 years ago

For the complete fix for EOF error, my plan is to signal a "EOF reached while reading from python's output stream" python-output-eof condition. This will have a "default" handler in stream-reload-string.

This will be coupled by a dynamic variable, which if bound or set to t would indicate that the caller is handling this condition and so stream-reload-string should resignal the condition. (Don't know if there's a cleaner work-around.)

In callpython.lisp, an additional macro do-with-retry/etc will help bind the condition handler for this to something that allows continuation - simply restart the process and retry the command.

Edit: Tried a more minimal approach of catching the error in dispatch-messages and then resignalling it.