slaclab / pydm

Python Display Manager
http://slaclab.github.io/pydm/
Other
112 stars 77 forks source link

PyDMShellCommand redirect output issues with no tty #1039

Open ZLLentz opened 1 year ago

ZLLentz commented 1 year ago

Describe the bug

I'm willing to implement a fix here, but I'd like some discussion/guidance first.

I noticed that some of our shell command buttons would "stop working", because our scientists reported that our applications wouldn't launch. I spent some time digging into it and figured out the following:

Expected behavior

Steps to Reproduce

  1. Create a python script that prints something and then also does some non-printing action you can check later, such as writing to a file or opening a gui. (Note: I haven't fully figured out which kinds of scripts here trigger the issue, but pydm guis themselves as subprocesses seem to be the most common offenders.)
  2. Create a ui file that contains a PyDMShellCommand that runs your script with redirectCommandOutput set to True (checked)
  3. Create a launcher script to help start and orphan the pydm process. It should be a shell script and contain a line like pydm my_file.ui &
  4. Open a new terminal on the machine that will run the pydm process. This can either done locally while sitting at the computer or remotely using e.g. xterm to launch a local terminal in a new window.
  5. In this new terminal, run your launcher script
  6. Close the terminal once the screen has appeared
  7. Click the button and verify that the non-printing action did not complete

Possible Solution

I had a few things in mind but I want other opinions: - Redirect the subprocess stdout to a function we control, rather than directly to stdout, so we can do some error handling - Rework this widget to always store the output locally in a way that we can check at runtime (separate from other terminal output) **My Platform**

OS red hat 7 pydm v1.19.1 python v3.9.15 pyqt v5.12.3

Additional context

jbellister-slac commented 1 year ago

Thanks for digging into this and managing to figure all that out! I should have a chance to reproduce this near the end of today so I can fully understand, and can let you know what I think afterwards.

ZLLentz commented 1 year ago

Thank you! Note that, locally, we're avoiding this now by "not doing that" e.g. not redirecting the print output to the terminal, so I don't consider this to be an "urgent" edge case per se. It can also be avoided by changing the way we launch our screens. But in general, it felt like something we could also do something about on the PyDM side.