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

Add support to disable concurrency check for UIScreen #111

Conversation

jkonecny12
Copy link
Member

Right now it is possible to disable concurrency check for InputHandler. However, this class is created on demand in the InputManager used by the UIScreen which means that a users are not able to disable concurrency check without re-implementation of the InputManager and InputHandler which is really cumbersome.

To solve this I'm adding a new property to InputManager and enable obtaining the InputManager instance from the UIScreen.

The concurrency check shouldn't be disabled if you don't have a strong reason to do it. It was implemented because without the check it's really hard to debug when multiple screens are asking for the input at once. The main reason to disable it is for error reporting after the application has crashed.

Closes #110

Copy link
Contributor

@poncovka poncovka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Thank you!

Right now it is possible to disable concurrency check for InputHandler. However,
this class is created on demand in the InputManager used by the UIScreen which
means that a users are not able to disable concurrency_check without
re-implementation of the InputManager and InputHandler which is really cumbersome.

To solve this I'm adding a new property to InputManager and enable obtaining the
InputManager instance from the UIScreen.

The concurrency check shouldn't be disabled if you don't have a strong reason to
do it. It was implemented because without the check it's really hard to debug
when multiple screens are asking for the input at once. The main reason to disable it
is for error reporting after the application has crashed.

Related: rhbz#1807491
This small change is done to simplify testing of the code.
@jkonecny12 jkonecny12 force-pushed the master-support-disable-concurrent-check-for-UIScreen branch 2 times, most recently from 271cf1d to 2d70d26 Compare October 8, 2021 11:46
@jkonecny12
Copy link
Member Author

UPDATED:

  • Rebased to fix pylint issues.
  • Fix test class bad argument name from the parent class.

@jkonecny12 jkonecny12 force-pushed the master-support-disable-concurrent-check-for-UIScreen branch from 2d70d26 to 741ae90 Compare October 8, 2021 11:48
@jkonecny12
Copy link
Member Author

UPDATED:

  • Fix tests by removing unused import.

@jkonecny12 jkonecny12 merged commit af06456 into rhinstaller:master Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting skip_concurrency_check from ErrorDialog
3 participants