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: 4074 - added explicit isolate/ui settings #4080

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

monsieurtanuki
Copy link
Contributor

What

  • In some background tasks we manipulate pictures, which can be costly performance wise. That's why we recently switched to isolates, for some specific operations.
  • For some reason the "isolate" part was not working anymore - or maybe it never worked on some specific cases.
  • Looking back at the doc, the question was "how could it work before?".
  • Anyway, now it works, and I even added an additional safety test, running the same method as non-isolate if relevant.
  • For my tests it was useful to add a "refresh" button on top of the background task page (typically in order to run again tasks in error), and I think we should keep that button.

Screenshot

New refresh button on top of background task page:
Screenshot_2023-06-05-15-53-27

Fixes bug(s)

Impacted files

  • image_compute_container.dart: added explicit isolate/ui settings
  • offline_tasks_page.dart: added a "refresh" button on the app bar

Impacted files:
* `image_compute_container.dart`: added explicit isolate/ui settings
* `offline_tasks_page.dart`: added a "refresh" button on the app bar
@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2023

Codecov Report

Merging #4080 (9f22858) into develop (6c292c0) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #4080      +/-   ##
===========================================
- Coverage    10.93%   10.92%   -0.02%     
===========================================
  Files          271      271              
  Lines        13454    13470      +16     
===========================================
  Hits          1471     1471              
- Misses       11983    11999      +16     
Impacted Files Coverage Δ
...mooth_app/lib/helpers/image_compute_container.dart 0.00% <0.00%> (ø)
...kages/smooth_app/lib/pages/offline_tasks_page.dart 1.81% <0.00%> (-0.19%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@teolemon teolemon requested a review from g123k June 5, 2023 14:53
@alexgarel
Copy link
Member

The refresh button is a good idea :-)

@monsieurtanuki
Copy link
Contributor Author

The refresh button is a good idea :-)

I had to fix the bug too, or else you would only get refreshed errors ;)
Btw the refresh is somehow automatic anyway, e.g. if you go to a product page.

@monsieurtanuki monsieurtanuki merged commit 2db6770 into openfoodfacts:develop Jun 5, 2023
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.

Image are not sent (Isolate/Background related)
4 participants