Run Upgrader in a GUI thread #6559
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ichorid
force-pushed
the
refactor/extract_upgrader
branch
from
ce5f16b to
8d2ed47
Compare
|
retest this please |
ichorid
changed the title
|
retest this please |
1 similar comment
|
retest this please |
ichorid
force-pushed
the
refactor/extract_upgrader
branch
2 times, most recently
from
ac128e3 to
8f62777
Compare
ichorid
requested review from
a team,
kozlovsky,
drew2a and
devos50
and removed request for
a team
drew2a
suggested changes
Nov 18, 2021
There was a problem hiding this comment.
This PR definitely looks like an improvement.
I've not reviewed the logic, because I don't understand it. I Hope @kozlovsky or @devos50 can do it properly.
I left some comments.
Also, pylint and coverage checks should be satisfied.
ichorid
force-pushed
the
refactor/extract_upgrader
branch
from
655ee35 to
bf30beb
Compare
|
@drew2a , the remaining lines will be very hard to cover. In fact, this PR increases the coverage - the lines that are shown as non-covered were never covered before (they just count as new lines, because these were extracted from CoreManager). |
kozlovsky
suggested changes
Nov 19, 2021
ichorid
force-pushed
the
refactor/extract_upgrader
branch
from
bc2dbdf to
877336e
Compare
kozlovsky
previously approved these changes
Nov 19, 2021
ichorid
force-pushed
the
refactor/extract_upgrader
branch
6 times, most recently
from
e46e571 to
7bef4e4
Compare
ichorid
force-pushed
the
refactor/extract_upgrader
branch
from
7bef4e4 to
6db4c11
Compare
|
retest this please |
kozlovsky
suggested changes
Nov 22, 2021
ichorid
force-pushed
the
refactor/extract_upgrader
branch
2 times, most recently
from
d4625b3 to
6fa367b
Compare
devos50
reviewed
Nov 22, 2021
ichorid
force-pushed
the
refactor/extract_upgrader
branch
from
6fa367b to
657298d
Compare
ichorid
force-pushed
the
refactor/extract_upgrader
branch
from
657298d to
3b4cb1a
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
devos50
approved these changes
Nov 22, 2021
kozlovsky
approved these changes
Nov 22, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Partially fixes #6525
This one moves some of the functions of Core Manager into the new Upgrade Manager class. The upgrader is now run in a GUI QThread for simplicity. Also, this changes
run_tribler.pyto use arguments instead of environment variables to indicate the running mode (Core vs GUI vs Core Test Mode). Arguments are safer and unambiguous.The main point of this PR is to stop sending event notifications from the Core from upgrader and thus pave the road for simplification of Tribler startup.
Upgrade Manager is briefly run during GUI tests, quitting when not able to find an existing Tribler state dir. This is not a real test, but at least it covers all the relevant lines. The downside is that when running single tests from the GUI suite waiting for additional time could be a bit annoying.
Simplification of GUI tests uncovered some hidden race conditions, which required minor fixes.