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

fix(files): Move download/upload to new tasks #1727

Merged
merged 15 commits into from
Jan 22, 2024
Merged

fix(files): Move download/upload to new tasks #1727

merged 15 commits into from
Jan 22, 2024

Conversation

Flemmli97
Copy link
Collaborator

@Flemmli97 Flemmli97 commented Jan 17, 2024 •
edited by dariusc93
Loading

What this PR does 📖

  • This PR changes uploads and downloads for both chat and files to be multithreaded:
    Each time a file is uploaded or downloaded a new task is made that deals with it.
    This fixes the problem where uploading/downloading a file will stop the whole app till the file is fully done.

@github-actions github-actions bot added Don't merge yet DO NOT MERGE Missing dev review Still needs to be reviewed by a dev labels Jan 17, 2024
@github-actions github-actions bot added the linter failing Cargo Workflow (linter) failed on this PR label Jan 17, 2024
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 17, 2024 •
edited
Loading

UI Automated Test Results Summary for MacOS/Windows

490 tests   438 ✅  2h 32m 11s ⏱️
 42 suites   52 💤
  3 files      0 ❌

Results for commit 51de925.

♻️ This comment has been updated with latest results.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 17, 2024 •
edited
Loading

UI Automated Tests execution is complete! You can find the test results report here

@dariusc93 dariusc93 changed the title fix(files): Move download/upload to new threads fix(files): Move download/upload to new tasks Jan 18, 2024
dariusc93
dariusc93 reviewed Jan 18, 2024
View reviewed changes
ui/src/utils/async_task_queue.rs Show resolved Hide resolved
ui/src/utils/async_task_queue.rs Outdated Show resolved Hide resolved
ui/src/utils/async_task_queue.rs Outdated Show resolved Hide resolved
ui/src/utils/async_task_queue.rs Show resolved Hide resolved
@github-actions github-actions bot removed the linter failing Cargo Workflow (linter) failed on this PR label Jan 18, 2024
@dariusc93 dariusc93 added the Ready for testing Ready for QA to test label Jan 18, 2024
@github-actions github-actions bot added linter failing Cargo Workflow (linter) failed on this PR Failed Automated Test This PR is failing Luis's Appium test and needs revised labels Jan 18, 2024
@luisecm
Copy link
Contributor

luisecm commented Jan 18, 2024

Tests are detecting an issue on build process:
image

@github-actions github-actions bot added the missing fixing conflict A conflict exists in current PR and needs resolution label Jan 18, 2024
@stavares843 stavares843 removed the Ready for testing Ready for QA to test label Jan 18, 2024
@github-actions github-actions bot removed linter failing Cargo Workflow (linter) failed on this PR Failed Automated Test This PR is failing Luis's Appium test and needs revised labels Jan 19, 2024
@Flemmli97 Flemmli97 removed the missing fixing conflict A conflict exists in current PR and needs resolution label Jan 19, 2024
stavares843
stavares843 requested changes Jan 19, 2024
View reviewed changes
Copy link
Member

@stavares843 stavares843 left a comment

Choose a reason for hiding this comment

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

this branch

Gravacao.do.ecra.2024-01-19.as.19.19.32.mov

vs dev

Gravacao.do.ecra.2024-01-19.as.19.22.36.mov

STR:

  • go to files
  • bulk upload 20 files for example
  • while is uploading the files number in queue is always the same even is really uploading one by one
  • going to dev, using the same files and the same flow, the counter works properly goes from 22, 21, 20 and so on

macOS, m1

@stavares843 stavares843 added the Changes requested When other dev or QA request a change label Jan 19, 2024
@Flemmli97 Flemmli97 removed the Changes requested When other dev or QA request a change label Jan 22, 2024
@Flemmli97
Copy link
Collaborator Author

fixed.
but due to parallel upload processing ui for files upload will flicker cause they all update at the same time.
personally would do a ui rework though on another PR or something

@stavares843
Copy link
Member

got it, thanks for fixing 🔨

@github-actions github-actions bot added the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Jan 22, 2024
@stavares843 stavares843 removed the Failed Automated Test This PR is failing Luis's Appium test and needs revised label Jan 22, 2024
@phillsatellite phillsatellite added the QA Tested QA has tested and approved label Jan 22, 2024
@dariusc93 dariusc93 added Ready to Merge This PR is ready to merge and removed Missing dev review Still needs to be reviewed by a dev Don't merge yet DO NOT MERGE labels Jan 22, 2024
@dariusc93 dariusc93 merged commit 7853af5 into dev Jan 22, 2024
10 checks passed
@dariusc93 dariusc93 deleted the up_download_handler_rework branch January 22, 2024 18:58
@github-actions github-actions bot removed QA Tested QA has tested and approved Ready to Merge This PR is ready to merge labels Jan 22, 2024
@Flemmli97 Flemmli97 mentioned this pull request Jan 22, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@phillsatellite phillsatellite phillsatellite approved these changes

@stavares843 stavares843 stavares843 approved these changes

@dariusc93 dariusc93 dariusc93 approved these changes

Assignees

@Flemmli97 Flemmli97

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Flemmli97 @luisecm @stavares843 @dariusc93 @phillsatellite