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

[devel] Core is importing from PyQt5 #5799

Closed
devos50 opened this issue Dec 4, 2020 · 6 comments · Fixed by #5852, #5870 or #5873
Closed

[devel] Core is importing from PyQt5 #5799

devos50 opened this issue Dec 4, 2020 · 6 comments · Fixed by #5852, #5870 or #5873
Labels
type: enhancement
Milestone
Backlog

Comments

@devos50
Copy link
Contributor

devos50 commented Dec 4, 2020

Minor issue, but I found that the core is importing Qt stuff through the packages in tribler-common:

Traceback (most recent call last):
  File "/Users/martijndevos/Documents/tribler/src/tribler-core/run_bandwidth_crawler.py", line 20, in <module>
    from tribler_core.session import Session
  File "/Users/martijndevos/Documents/tribler/src/tribler-core/tribler_core/session.py", line 23, in <module>
    from tribler_common.sentry_reporter.sentry_reporter import SentryReporter
  File "/Users/martijndevos/Documents/tribler/src/tribler-common/tribler_common/sentry_reporter/sentry_reporter.py", line 7, in <module>
    from PyQt5.QtWidgets import QApplication, QMessageBox

When running the core, I would like to avoid importing from PyQt5 as much as possible.
@devos50 devos50 added this to the Backlog milestone Dec 4, 2020
@devos50 devos50 changed the title [devel] Core is importing Qt stuff [devel] Core is importing from PyQt5 Dec 4, 2020
@ichorid
Copy link
Contributor

ichorid commented Dec 4, 2020

Yep, totally agree. The Core should be completely independent of the QT stuff.

@ichorid
Copy link
Contributor

ichorid commented Dec 4, 2020

@drew2a , could you come up with some way to move the stuff to e.g. run_tribler.py?

@drew2a
Copy link
Collaborator

drew2a commented Dec 4, 2020

QMessageBox needs to confirm sending unhandled errors without interaction with GUI.

@drew2a
Copy link
Collaborator

drew2a commented Dec 4, 2020 •
edited
Loading

Two cases:

  1. An error had occurred before core-GUI interaction has been established.
  2. An error has been occurred in self.unhandled_error_observer.

@Dmole
Copy link
Contributor

Dmole commented Dec 6, 2020

QMessageBox needs to confirm sending unhandled errors without interaction with GUI.

It really need not.
Have a setting to auto send or not in the config.
And/or dump it in the log file (assuming it's not already, that's what it's for)

@devos50
Copy link
Contributor Author

devos50 commented Dec 7, 2020

It now also requires the DAS5 nodes to have PyQt5 installed:

INFO:ProcessRunner:[] ERR: 2020-12-07 04:08:56,575:INFO:All subscribers connected!
INFO:ProcessRunner:[das4_reserve_and_run...] OUT: 2020-12-07 04:08:56,574:INFO:Scenario file found, using /var/scratch/pouwelse/jenkins2/workspace/validation_experiments/validation_experiment_dht/gumby/experiments/dht/dht_experiment.das4.scenario
INFO:ProcessRunner:[das4_reserve_and_run...] OUT: 2020-12-07 04:08:57,095:INFO:Unable to load gumby.modules.tribler_module as a local module, exception: No module named 'PyQt5'
INFO:ProcessRunner:[das4_reserve_and_run...] OUT: 2020-12-07 04:08:57,098:ERROR:Unable to import tribler_module from /var/scratch/pouwelse/jenkins2/workspace/validation_experiments/validation_experiment_dht/gumby/gumby/modules as gumby.modules.tribler_module: No module named 'PyQt5'

@drew2a when running Tribler in headless mode, I do not want to install a GUI library only for a single dialog that only triggers when an error occurs. Note that it is also likely to fail when running Tribler on (headless) remote servers without VNC installed. Can we/do we want to send these reports automatically, without showing a dialog?

@drew2a drew2a linked a pull request Dec 16, 2020 that will close this issue
Convert PyQt5 to local import in tribler common #5852
Merged
This was referenced Dec 18, 2020
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: enhancement
Projects
None yet
Milestone
Backlog
4 participants
@devos50 @Dmole @ichorid @drew2a