return dummy files on error and update in the background. #76
Conversation
… background updating
|
Ooh, I like this. I'll test it this evening. |
|
@drinfernoo tested on the case where cache is empty and it seems to work well. |
|
@drinfernoo btw I think I left a comment in the code. its not clear what should happen when we get an error in a path. At the moment if we have a cached version we just keep showing that. |
|
I still need to take a look, but unfortunately I ran out of time to test tonight, so I may not get to it for a few days 😬 |
|
I'm also seeing that this breaks exploding/cloning, as the only item it sees is EDIT: This may have been broken since caching was introduced... I don't know that I actually tested it before. |
Which part does it break? The initial adding of an exploded folder because it can't get the names of the sub paths?
Possibly not because before it either returned the old cached version or blocked and returned the latest version, both of which had proper names and items. |
|
Not sure exactly what's happening, but it only adds 1 item to the exploded group: "Loading Items..." . |
…uild the group properly on add
|
@drinfernoo I think I fixed the exploding problem. |
|
I actually have an issue for that already 😉 #34 |
|
Not sure whats going on here. Note it down to look at later |
|
@djay Do you have a merged widget? Or perhaps one showing a different page? |
|
As a test of the "error" state, I exploded the I'm now stuck in a loop of the same list trying to be loaded over and over again: Forcing a widget refresh didn't help, as it's now just trying to load a different list: Reauthorizing Trakt technically helped, but I had to manually clear the cache files in order for it reload the widget that should've been showing... and I was completely unable to trigger an error state. Something's still not quite right 😅 |
|
It also appears that the code in In any case, I think the final snag for this PR is to get it to realize that when there are no items returned, we should be in an "error" state, and stop trying to read the cache, rather than trying over and over again. |
|
@drinfernoo if there is a cache and an error is returned then it doesn't return a error tile. It returns the cache. This makes sense for temporary problems but perhaps not for longer term issues. I highlighted this above. We could return an error but then its a pain for a temp issue (like throttling). We could also add an error tile to the start of the path to show there was a problem? What do you think? |
|
Still seeing looping with failed widgets... I added a handful of my Trakt paths from TMDbHelper, and then deauthorized my account. This is making TMDbHelper throw 401 errors, and unable to load these lists, but AutoWidget just keeps on trying 😅 I think if after... maybe three (?) retries with no path returned, let's just return the error state. |
|
Hey, I think it would be a good time to try to punch these through 😉 |
drinfernoo
left a comment
drinfernoo
left a comment
There was a problem hiding this comment.
I just realized that I never actually submitted this review 😅
|
I've added a rudimentary "error state" for widgets that end up with an error... but it is admittedly very basic and could use some help: af7d6fd |
|
I've gone ahead and done the rebase (as far as I can tell, still needs testing) in my I'm gonna have some people give this some testing, but I'd like to get this one merged soonish. |
|
I also went ahead and resolved my conversations, as I'm not sure they actually apply anymore... been a long time since I made that review. |
|
Definitely have some issue here... likely due to the way I changed |
|
I'm not sure how we can best collaborate on this now... I have a branch and you have a branch 😅 I'm trying to get the pre-caching working for paged widgets, but I think I'm missing something, or possibly goofed up the rebase. I'm never getting a |
|
@drinfernoo not sure. hard to remember what I was doing. brief look at my code there is more stuff in there than handling errors such as switching to background progress. |
|
@djay, What I'm seeing when I step through, is that: ends up calling through to: But that touch method doesn't seem to actually be doing anything 🤔
No |
|
@drinfernoo could be a platform specific thing? I only ever tested on mac and android. Would certainly help to have automated tests for this stuff so easy to run on different platforms and see which bit is breaking. |
|
@drinfernoo https://stackoverflow.com/questions/42630281/utime-has-no-effect-in-windows One thing that would be good to do is put in an assert statement in touch that checks to make sure it worked |
After making some changes like they showed in that SO thread, something still wasn't working out... I tried doing this: def push_cache_queue(hash, widget_id=None):
queue_path = os.path.join(_addon_data, "{}.queue".format(hash))
history = read_history(hash, create_if_missing=True) # Ensure its created
if widget_id is not None and widget_id not in history["widgets"]:
history_path = os.path.join(_addon_data, "{}.history".format(hash))
history["widgets"].append(widget_id)
write_json(history_path, history)
if os.path.exists(queue_path):
pass # Leave original modification date so item is higher priority
else:
write_file(queue_path, hash) # write the file instead of only touching
# touch(queue_path)And now I can get a file created, but I'm having trouble figuring out how to get them to be used properly. This caching stuff is a bit beyond me, and I'm just trying to wade through what you set up 😉 I did find that a small change needed to be made in getting the path from the widget, since I've now changed the widget format from holding a full copy of the path, to only its ID: dd87fde |
|
Closing this in favor of #98, as it's much more up-to-date and shouldn't have as many conflicts 😅 |
2 participants
and Notify when background updating