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 GUI downloads status update #7319

Merged
merged 4 commits into from Mar 14, 2023
Merged

Fix GUI downloads status update #7319

merged 4 commits into from Mar 14, 2023

Conversation

drew2a
Copy link
Collaborator

@drew2a drew2a commented Mar 14, 2023

This PR fixes #7318

The biggest part of this PR is a refactoring of DLSTATUS. This change introduces Status(Enum) which makes it possible to write stricter code and reduce duplication of the status definitions.

Also, a new field was added to json response of async def get_downloads(self, request) to reduce unnecessary conversions from a status code to a status string and back. In simple words this change makes it possible to remove the following function:

    def get_raw_download_status(self):
        return dlstatus_strings.index(self.download_info["status"])

The bugfix itself is the following change (simplified):

from:

    def on_download_stopped(self, json_result):
        if json_result and "modified" in json_result:
            for selected_item in self.selected_items:
                if selected_item.download_info["infohash"] == json_result["infohash"]:
                    selected_item.download_info['status'] = "DLSTATUS_STOPPED"
                    selected_item.update_item()
                    self.update_downloads()

to:

    def on_download_stopped(self, json_result):
        if json_result and "modified" in json_result:
            for selected_item in self.selected_items:
                if selected_item.download_info["infohash"] == json_result["infohash"]:
                    selected_item.update_item()
                    self.update_downloads()

The root cause of the bug is the resynchronization of a download state between the GUI and the core.

@drew2a drew2a force-pushed the fix/7318 branch 3 times, most recently from 016a1c1 to e4b7054 Compare March 14, 2023 13:54
@drew2a drew2a marked this pull request as ready for review March 14, 2023 14:16
@drew2a drew2a requested a review from a team as a code owner March 14, 2023 14:16
@drew2a drew2a requested review from kozlovsky and removed request for a team March 14, 2023 14:16
Copy link
Collaborator

@kozlovsky kozlovsky 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; I only have minor comments about naming

src/tribler/gui/widgets/downloadspage.py Outdated Show resolved Hide resolved
src/tribler/core/utilities/simpledefs.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@kozlovsky kozlovsky left a comment

Choose a reason for hiding this comment

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

The DLSTATUS_XXX statuses are also described in the doc/restapi/introduction.rst file, probably should be renamed here as well. Also, I see they are mentioned in comments in DownloadState.get_progress & get_status

@drew2a drew2a merged commit af701fe into Tribler:main Mar 14, 2023
15 checks passed
@drew2a drew2a deleted the fix/7318 branch March 14, 2023 17:11
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.

Download start: statuses switching in unexpected order
2 participants