Fixed daemon detection to determine forking behavior #3140
Conversation
Introduced the `is_daemon_process` function in `utils/system.py` to determine if the current process is a daemon. Updated the logic in `constants.py` to use this new utility function for determining forking behavior.
|
|
|
Hey @sammywachtel ! Thanks a lot for your submission. Can you please provide a little more context around the issue you ran into with the existing implementation and how the new approach is better. |
sqlmesh/utils/system.py
Outdated
| :rtype: bool | ||
| """ | ||
|
|
||
| return not os.isatty(sys.stdout.fileno()) |
There was a problem hiding this comment.
Can't you just do:
sys.stdout.isatty()
?
There was a problem hiding this comment.
Hey @izeigerman. I believe there's a risk of getting an AttributeError in some cases when the underlying file descriptor doesn't implement isatty(). Or if the stdout is in a wrapper that doesn't correctly implement isatty(). Anyway, it feels like best practice. It would work most of the time with just sys.stdout.isatty(), though.
There was a problem hiding this comment.
As for your question about why the change: I am working with the airflow integration and after one of the recent new versions (I was on 0.18.0 I think) I started getting errors surrounding the worker settings. I noticed the code in constants.py had a recent change that was supposed restrict the number of workers to 1 when in an airflow environment, but the check for the process being within a daemon was using the multiprocessing package which would only return a "true" if the daemon were a multiprocessing package daemon. It doesn't appear to be that way with airflow, but rather, it's using a standard system daemon for which there's no built-in check. I hope that helps!
This change eliminates the `system.py` file and integrates the `is_daemon_process` function into `constants.py`.
|
@sammywachtel please sign the cla, thank you! |
It is signed, but for some reason still shows up as pending. |
Introduced the
is_daemon_processfunction inutils/system.pyto determine if the current process is a daemon. Updated the logic inconstants.pyto use this new utility function for determining forking behavior.