comictagger / metron_talker

Metron.cloud comictagger talker plugin
3 stars 1 forks source link

Mokkari sqlite cache #2

Closed lordwelch closed 10 months ago

lordwelch commented 10 months ago

Is there a particular reason you do not use mokkari's sqlite cache? You can replace almost all instances of mokkari.api(self.username, self.user_password, user_agent="comictagger/" + self.version) with mokkari.api(self.username, self.user_password, cache=mokkari.sqlite_cache.SqliteCache(self.cache_folder/"metron.db", expire=7), user_agent="comictagger/" + self.version) and it will dramatically simplify the code

mizaki commented 10 months ago

How would CT be able to clear the cache? Specifically in the settings window.

lordwelch commented 10 months ago

We would either give the full path to the cache and have it use the same sqlite db or just remove the entire folder

mizaki commented 10 months ago

The problem occurs when needing to manipulate the result.

The series cover URL is injected if the option is set.

To cause the issue window to retrieve the full issue information (title, desc, variant covers etc. is missing from IssueList), the talker removes the image URL.

As far as I can tell (and why would you normally want to), there is no way to get between the mokkari cache and the API result.

mizaki commented 10 months ago

Of course, if it uses the cache it doesn't matter if extra API calls are done after. Doing that should still make it easier.

mizaki commented 10 months ago

I've switched it over and it looks a lot simpler. The only issue is that the extra info isn't cached so publisher, issue titles aren't populated but that relates to https://github.com/comictagger/comictagger/issues/512

lordwelch commented 10 months ago

I've switched it over and it looks a lot simpler. The only issue is that the extra info isn't cached so publisher, issue titles aren't populated but that relates to comictagger/comictagger#512

Even when that's fixed it will only update after a user selects the issue which means if they want to search through all of the titles they have to load every single issue before using the filter... which is not ideal. On the other hand even with the previous caching in place they would still have to do something to retrieve each full issue once to do that so there is only a very small loss in functionality.

mizaki commented 10 months ago

Addressed this a little in https://github.com/mizaki/comictagger-metron-talker/issues/5 but it is a limitation of the API and there is no fix other than from the API end (I'll raise the issue with Metron but https://github.com/bpepple/metron/discussions/164#discussioncomment-5948168). Doing a full fetch for each issue is also not an option so the filter field with Metron as is, is currently next to useless.

A hybrid approach might be to cache the issue data only in the CT cache and then that could be used "on top" if it's available which would get us back to where we were.

lordwelch commented 10 months ago

Now we know the limitations of mokkari's cache and why it isn't the best fit for the CT usecase