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.

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

Merged

Conversation

romainf-ubi
Copy link
Contributor

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:

$ 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

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
```
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 8, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@romainf-ubi
Copy link
Contributor Author

I still hope my company will approve the CLA soon 😅

Copy link
Collaborator

@bcipriano bcipriano left a 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.

@romainf-ubi
Copy link
Contributor Author

/easycla

@romainf-ubi
Copy link
Contributor Author

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.

@romainf-ubi
Copy link
Contributor Author

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

@bcipriano
Copy link
Collaborator

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.

@bcipriano bcipriano merged commit 3b2326e into AcademySoftwareFoundation:master Jan 12, 2023
@romainf-ubi
Copy link
Contributor Author

Yeah, sorry, I tried to merge the latest master to see if it would fix the issue, but didn't think it would remove the previous logs.

Anyway, merging seems to have solved the issue! 😉

@romainf-ubi romainf-ubi deleted the upstream-pynput branch January 12, 2023 17:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RQD complains about missing package pynput
2 participants