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.

Sign up for GitHub

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

Jump to bottom

flupy CLI does not work on Windows #33

Open
abraha2d opened this issue Apr 12, 2024 · 6 comments · May be fixed by #34
Open

flupy CLI does not work on Windows #33

abraha2d opened this issue Apr 12, 2024 · 6 comments · May be fixed by #34

Comments

@abraha2d
Copy link

Ran into this issue while in the process of adding flupy (and alembic-utils) to conda-forge here: conda-forge/staged-recipes#26023.

Seems to be due to this bit here:

flupy/src/flupy/cli/cli.py

Line 4 in 5fc989c

from signal import SIG_DFL, SIGPIPE, signal

signal.SIGPIPE is not available on Windows.

Traceback:

>flu --help 
Traceback (most recent call last):
  File "C:\bld\flupy_1712887240528\_test_env\Scripts\flu-script.py", line 5, in <module>
    from flupy.cli.cli import main
  File "C:\bld\flupy_1712887240528\_test_env\Lib\site-packages\flupy\cli\cli.py", line 4, in <module>
    from signal import SIG_DFL, SIGPIPE, signal
ImportError: cannot import name 'SIGPIPE' from 'signal' (C:\bld\flupy_1712887240528\_test_env\Lib\signal.py). Did you mean: 'SIGFPE'?
@abraha2d abraha2d mentioned this issue Apr 12, 2024
Merged
10 tasks
@olirice
Copy link
Owner

olirice commented Apr 12, 2024

Unfortunately I don't have access to a windows machine. Any chance you'd be interested in a creating a PR?

@abraha2d
Copy link
Author

Let me see if I can put up a PR. I also don't run Windows regularly, but do occasionally use a VM.

@abraha2d
Copy link
Author

Looked into this a little more... I think the solution might be to just remove that line entirely?
SIGPIPE is already ignored in Python by default: https://docs.python.org/3/library/signal.html#signal.SIGPIPE

@olirice
Copy link
Owner

olirice commented Apr 26, 2024 •
edited

did you get a chance to test if that functioned on a windows machine, or is that a best guess?

@abraha2d
Copy link
Author

Not tested on Windows (still trying to recover my VM), but I did test on Linux, and removing that line did not change any behavior.

Tested it with this invocation:
journalctl -ef | flu '_.filter(lambda x: x.startswith("Apr 27 04:15"))'
(and then finding the PID of the journalctl process and killing it)

No broken pipe exception was thrown. This was with Python 3.10 on Ubuntu 22.04

@abraha2d abraha2d linked a pull request Apr 27, 2024 that will close this issue
Open
@abraha2d
Copy link
Author

abraha2d commented Apr 27, 2024 •
edited

Ok, verified that it fixes functionality on Windows as well:

(.venv) C:\git\flupy>type logs.txt
ERROR 1
ERROR 2
ERROR 3
NOT AN ERROR 4
ERROR 5
NOT AN ERROR 6
NOT AN ERROR 7
ERROR 8
NOT AN ERROR 9
NOT AN ERROR 10

(.venv) C:\git\flupy>type logs.txt | flu "_.filter(lambda x: x.startswith('ERROR'))"
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\git\flupy\.venv\Scripts\flu.exe\__main__.py", line 4, in <module>
  File "C:\git\flupy\src\flupy\cli\cli.py", line 4, in <module>
    from signal import SIG_DFL, SIGPIPE, signal
ImportError: cannot import name 'SIGPIPE' from 'signal' (C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.12_3.12.1008.0_x64__qbz5n2kfra8p0\Lib\signal.py). Did you mean: 'SIGFPE'?

(.venv) C:\git\flupy>git checkout fix-windows-compat
branch 'fix-windows-compat' set up to track 'origin/fix-windows-compat'.
Switched to a new branch 'fix-windows-compat'

(.venv) C:\git\flupy>type logs.txt | flu "_.filter(lambda x: x.startswith('ERROR'))"
ERROR 1
ERROR 2
ERROR 3
ERROR 5
ERROR 8

#34 is now ready for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix Windows compatibility abraha2d/flupy
2 participants
@abraha2d @olirice