-
Notifications
You must be signed in to change notification settings - Fork 201
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
fix(rqd): add pynput in requirements #1230
fix(rqd): add pynput in requirements #1230
Conversation
When launching RQD, there was these errors complaining of the missing pynput package: ```text $ CUEBOT_HOSTS="localhost" rqd WARNING:root:CUEBOT_HOSTNAME: localhost WARNING:root:RQD Starting Up 2022-12-08 10:04:39,643 WARNING openrqd-__main__ RQD Starting Up ERROR:rqd.rqnimby:Failed to import pynput, falling back to Select module Traceback (most recent call last): File "D:\dev\opencue\opencue\venv\lib\site-packages\rqd-unknown-py3.10.egg\rqd\rqnimby.py", line 55, in getNimby import pynput ModuleNotFoundError: No module named 'pynput' 2022-12-08 10:04:39,644 ERROR openrqd-rqnimby Failed to import pynput, falling back to Select module Traceback (most recent call last): File "D:\dev\opencue\opencue\venv\lib\site-packages\rqd-unknown-py3.10.egg\rqd\rqnimby.py", line 55, in getNimby import pynput ModuleNotFoundError: No module named 'pynput' WARNING:rqd.rqnimby:Using Nimby nop! Something went wrong on nimby's initialization. 2022-12-08 10:04:39,652 WARNING openrqd-rqnimby Using Nimby nop! Something went wrong on nimby's initialization. WARNING:rqd.rqcore:RQD Started 2022-12-08 10:04:39,690 WARNING openrqd-rqcore RQD Started ``` By adding pynput in requirements.txt, all the errors are now gone: ```text $ CUEBOT_HOSTS="localhost" rqd WARNING:root:CUEBOT_HOSTNAME: localhost WARNING:root:RQD Starting Up 2022-12-08 09:21:13,889 WARNING openrqd-__main__ RQD Starting Up WARNING:rqd.rqcore:RQD Started 2022-12-08 09:21:13,958 WARNING openrqd-rqcore RQD Started ```
|
I still hope my company will approve the CLA soon 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change LGTM.
I added some more detail in #1229, but since pynput
is expected by default, I agree it should be included as a dependency.
/easycla |
EasyCLA is signed, but other jobs are still red. I took a look at the details, but I don't really understand the reason of the failure. |
I took a look at the ci python script: https://github.com/AcademySoftwareFoundation/OpenCue/blob/master/ci/check_changed_files.py But the PR doesn't touch any of the blacklisted files listed there |
I can't seem to find the logs from the previous run, so not sure what happened there. We made some CI fixes recently, maybe it was related. Sometimes you just need to merge master -- some of those scripts rely on Git history and can sometimes get confused if master is ahead of your branch. Passing now, anyway. |
Yeah, sorry, I tried to merge the latest Anyway, merging seems to have solved the issue! 😉 |
Link the Issue(s) this Pull Request is related to.
Fixes #1229
Summarize your change.
By adding pynput in requirements.txt, all the errors are now gone: