Finish precaching, background updates, and placeholder items by drinfernoo · Pull Request #98 · drinfernoo/plugin.program.autowidget

drinfernoo · 2021-07-23T15:11:10Z

This PR builds on code from #76, and largely covers precaching of paths which will be displayed in the future, like "Next Page" paths and others in the same Cycling Widget. It also adds three "placeholder" items that can appear during widget load:

"Loading content..." for when a widget is... loading content 😅
"No content found for X" for when no items are returned for a particular path
"Error showing X" for when the response for a path is invalid in some way

This PR also ensures that we don't save an invalid cache for the latter case, and adds some notifications about various background processes, like exploding paths and updating widgets in the background.

… background updating

…uild the group properly on add

… yet

… background updating

drinfernoo · 2021-07-23T16:22:51Z

                props=properties,
            )
+            # Ensure we precache next page for faster access
+            cache.cache_expiry(file["file"], widget_id, background=True)


Here's the magic of pre-caching a "Next Page" item.

drinfernoo · 2021-07-23T16:24:11Z

                props=properties,
            )
+            # Ensure we precache next page for faster access
+            cache.cache_expiry(file["file"], widget_id)


Here's the magic where we pre-cache the next page.

I think this means a precached next page is going to cause an unneeded widget redraw. Pass some flag in teh queue file to say not to refresh the widget if its a precache?

drinfernoo · 2021-07-23T16:24:43Z

-        return titles if titles else True, path_label, content

    utils.log("Loading items from {}".format(path), "debug")
+    files, hash = refresh.get_files_list(path, path_label, widget_id)


Passing path_label through this allows us to properly label the placeholder tiles.

drinfernoo · 2021-07-23T16:26:48Z

                    )  # Just in queued path's widget defintion has changed and it didn't update this path
                    unrefreshed_widgets = unrefreshed_widgets.union(
-                        effected_widgets
+                        affected_widgets


Had to fix these 😉

drinfernoo · 2021-07-23T16:27:20Z

+                    hash, widget_ids = queue.pop(0)
+                    utils.log("Dequeued cache update: {}".format(hash[:5]), "notice")
+
+                    affected_widgets = cache.cache_and_update(


This bit right here makes sure to cache any queued paths any time they show up.

drinfernoo · 2021-07-23T16:28:22Z

-def get_files_list(path, widget_id=None):
-    hash = utils.path2hash(path)
-    _, files, _ = utils.cache_expiry(hash, widget_id)
+def get_files_list(path, label=None, widget_id=None, background=True):


We always do caching in the background, except in the case of exploding/cloning.

I was thinking next page should not be done in the background but I guess if its always precached then this is no longer a problem

drinfernoo · 2021-07-23T16:28:41Z

+        # Should only happen now when background is False
        utils.log("Blocking cache path read: {}".format(hash[:5]), "info")
-        files, changed = utils.cache_files(path, widget_id)
+        files, changed = cache.cache_files(path, widget_id)


Create a cache for the path if there isn't one already.

drinfernoo · 2021-07-23T16:29:18Z

-    else:
        utils.log("Error processing {}".format(hash), "error")
-        return None, hash
+        error_tile = utils.make_holding_path(


Create an "error state" tile if the JSON response in the existing cache file is invalid in some way.

drinfernoo · 2021-07-23T16:30:08Z

+        cache_path = os.path.join(_addon_data, "{}.cache".format(hash))
+        if os.path.exists(cache_path):
+            os.remove(cache_path)
+        utils.log("Invalid cache file removed for {}".format(hash))


If that is the case, we'll go ahead and remove that cache file now, so that we don't hold an invalid cache.

drinfernoo · 2021-07-23T16:30:38Z

+
+    if not files:
+        utils.log("No items found for {}".format(hash))
+        empty_tile = utils.make_holding_path(


If the cache is valid, but there are no files to return, then we make an "empty path" tile instead.

drinfernoo · 2021-07-23T16:31:02Z

+    for file in files:
+        new_file = {k: v for k, v in file.items() if v is not None}

-def queue_widget_update(widget_id):


It didn't seem that this method was actually used anywhere, so I just got rid of it again.

yes. original way I tried to do queues. not used anymore but useful to know there is another way to communicate with the service process

drinfernoo

@djay I've commented up the PR a bit, so you can check and see if you think my changes are reasonable 👍

drinfernoo · 2021-07-23T16:34:44Z

 msgstr "No content found for {}"
+
+#: /resources/lib/refresh.py:95
+msgctxt "#30141"


I "viewed" all of the other .po files, but these are the new translations I added.

drinfernoo · 2021-07-23T16:37:25Z

The other thing that sticks out to me a lot in this code is that there are a lot of leftover commented bits and "todos", but those should probably be tackled in a further "refactor" PR focused on performance and flexibility of these caching processes.

…de_errors_bug_fix_edition

…tion Djay/hande errors bug fix edition

djay · 2021-08-02T05:54:26Z

I should probably go through the TODOs. Often they get fixed later somewhere else I forget I had a TODO for it.
e.g. I think this might be handled by your precache code right? on refresh a next cycled path is precached?

    # TODO: update paths on autowidget refresh based on predicted update frequency. e.g. plugins with random paths should
    # update when autowidget updates.

Some other TODOs might be better moved to github feature requests. Like periodic caching cleaning to reduce space.

I think most of the rest of the TODOs are around estimating how long to cache a path, or the likelyhood of a path needing updating after playback. Not sure if these are enabled in the latest code?

drinfernoo · 2021-08-10T13:43:39Z

I should probably go through the TODOs. Often they get fixed later somewhere else I forget I had a TODO for it.
e.g. I think this might be handled by your precache code right? on refresh a next cycled path is precached?
    # TODO: update paths on autowidget refresh based on predicted update frequency. e.g. plugins with random paths should
    # update when autowidget updates.
Some other TODOs might be better moved to github feature requests. Like periodic caching cleaning to reduce space.

I think most of the rest of the TODOs are around estimating how long to cache a path, or the likelyhood of a path needing updating after playback. Not sure if these are enabled in the latest code?

So, refreshing after playback is included in this PR, but it should also be in master as well. I'm honestly not sure if it's really working the way it should, especially with regards to the "forensic" caching you've got in here... but I'm going to say we can tackle that at a later date.

I'd like to go ahead and merge this in soon, and start looking at some other potential refactoring and performance increases for a future major update, which will likely include some of those caching mechanisms.

Also, for what it's worth, this PR moves most all of the caching stuff to cache.py for organization, so hopefully future maintenance of the caching system will be a little bit easier.

djay and others added 30 commits July 18, 2021 16:32

return dummy files on error and update in the background. Notify when…

ba5c2ac

… background updating

handle unicode in notification

cd94cd5

show when widgets have finished updating

afac5a9

notification when backround updating ends

89883ad

fix starting up with no cache

91503bf

don't do notifications duirng playback

6f54fda

and final notify

ce94676

tidy update dummy path generation

d140a9f

ensure explode and clone get the path in the foreground so they can b…

0a406e1

…uild the group properly on add

use progress to show background updating

12d6a51

add progress when exploding

f0ba810

copying instead of exploding

d9e52fd

reduce wait for queue processing to 1s.

9090597

precaching next page on paged widgets. but doesn't seem to be working…

1e44c9a

… yet

ensure we have history when queuing updates

85aa4c0

Merge branch 'djay/handle_errors' into handle_errors

0a616f7

🐛 - Fix addon data paths

7f0606d

🐛 - Try to fix paged widget caching

adad306

👽 - Add compatibility for old widgets

dd87fde

♻️ - Use last_read

3285612

💄 - Format with black

819247c

♻️ - Cache files in background

0bc55c3

return dummy files on error and update in the background. Notify when…

c6bce55

… background updating

handle unicode in notification

f938cc1

show when widgets have finished updating

e104d0f

notification when backround updating ends

e64bf99

fix starting up with no cache

17120b8

don't do notifications duirng playback

87c2cd7

and final notify

2d7dbfa

tidy update dummy path generation

09efe12

♻️ - No need to specify background

ac24a02

drinfernoo commented Jul 23, 2021

View reviewed changes

drinfernoo force-pushed the develop branch 2 times, most recently from a905d44 to 9542859 Compare July 26, 2021 12:25

djay and others added 5 commits July 26, 2021 22:10

only update requested path and with updating missing widget

b9c5e5f

Merge commit '4d0339ff2dbfaac95fd5ad83eacf8008f1c6847c' into djay/han…

6b9f2ba

…de_errors_bug_fix_edition

Merge commit '510ee2df6e3f117e2d5482e92ea83fc7fc1028ca' into djay/han…

d4277b8

…de_errors_bug_fix_edition

Merge pull request #101 from drinfernoo/djay/hande_errors_bug_fix_edi…

5f73680

…tion Djay/hande errors bug fix edition

Merge branch 'develop' into handle_errors

3e5b213

drinfernoo and others added 4 commits August 10, 2021 06:44

Merge branch 'develop' into handle_errors

05bc19a

🐛 - Fix refresh after playback

40dbccb

⚡ - Don't process cache queue during video playback

a2ed968

stop processing queue during playback

f706b98

drinfernoo mentioned this pull request Aug 26, 2021

[Bug] Group files will not automatically update #105

Closed

drinfernoo merged commit 5a950b1 into develop Sep 6, 2021

Conversation

drinfernoo commented Jul 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drinfernoo Jul 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drinfernoo left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

drinfernoo commented Jul 23, 2021

Uh oh!

djay commented Aug 2, 2021

Uh oh!

drinfernoo commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

drinfernoo Jul 23, 2021 •

edited

Loading

drinfernoo left a comment •

edited

Loading

drinfernoo commented Aug 10, 2021 •

edited

Loading