nilsnolde / wordpress-markdown-git

:loop: WordPress plugin to add file content (Markdown, Jupyter notebooks) from a Git based VCS to a WordPress post; replaces https://github.com/gis-ops/md-github-wordpress
GNU General Public License v3.0
48 stars 14 forks source link

Allowing content cache #19

Closed danilopinotti closed 4 years ago

nilsnolde commented 4 years ago

Woohoo! Thanks @danilopinotti . Will look into tonight. The wp_cache_x functions were the missing piece.. How could I overlook that...

Is it ok for you if I take the opportunity and commit to this PR with the steps outlined in #4 (i.e. HEAD request to determine cachability instead of static TTL; ideally let the user choose which strategy to use).

danilopinotti commented 4 years ago

Woohoo! Thanks @danilopinotti . Will look into tonight. The wp_cache_x functions were the missing piece.. How could I overlook that...

Is it ok for you if I take the opportunity and commit to this PR with the steps outlined in #4 (i.e. HEAD request to determine cachability instead of static TTL; ideally let the user choose which strategy to use).

What about in the next update?

Make external requests will slow down the posts loading... IMO it's better to make a webhook that cleans these caches. This webhook (authenticated or not) may be called using Github Actions or other automation tools.

nilsnolde commented 4 years ago

Webhooks would be really neat! Much easier than what I was thinking of.. A little high-barrier maybe for the casual WP user, but the gain is enormous (in terms of the little impact of this plugin generally;)). Looks like all providers support webhook auth. Good stufff, thanks.

Will amend this PR with caching for notebooks and test this afternoon.

nilsnolde commented 4 years ago

Digging a little more @danilopinotti , from documentation:

By default, the object cache is non-persistent. This means that data stored in the cache resides in memory only and only for the duration of the request.

So WP Object Cache doesn't help us. There's plugins to set up persistent cache, though most require some in-memory database which I wouldn't like to have as a dependency. This one could be ok, but also requires manual setup.. https://github.com/l3rady/WordPress-APCu-Object-Cache..

Another way is Transients, they're actually doing what you were aiming at.. No groups though, but that hardly matters.

nilsnolde commented 4 years ago

Yeah, I tried the get/set_transient and that works! Would be fine with me. Had more file-based in mind so it's easier to "flush" the cache manually in worst case. But this works too..

nilsnolde commented 4 years ago

Ah should've read the README I guess :D I really do prefer the path of least resistance, ie using the transient API. IMO there's no need for the lightning-fast in-mem dbs from other plugins.

Since I already tried it, I'll push a commit and you can try yourself. Also addressing my review comments.

danilopinotti commented 4 years ago

Hi @nilsnolde I didn't know the transient API. I think the less dependencies, the better. I fully agree with your update using Transient API.

Today I will test your update

danilopinotti commented 4 years ago

Hi! I updated the cache key generation strategy and it seems all ok!

Reading the code, I find another example to use whole sc_attrs as cache key: the 'limit' property. When we change the limit, the cache need to be updated...

I tested this update in a WP 5.5.1.

danilopinotti commented 4 years ago

@danilopinotti thanks again for your efforts here! And sorry for squashing all these changes in a single commit.. Let me know if you agree with these changes and hope you're not feeling like I override you too much. Without you I'm not sure if I'd taken the time to take a deeper look at this. It improves the value of this plugin so much!!

You can squash the commits without a problem. May you associate me as author or co-author on commit?

Anyway, the code got interesting and adaptable. I think we got a satisfactory version.

Thanks for enchancements and for accept my PR through constructive dialogue.

nilsnolde commented 4 years ago

You can squash the commits without a problem.

I meant my recent commit was not very targeted, all changes in a big commit. I won't squash, I think it's nicer towards contributors such as yourself. I don't care too much about history here ;)

Thanks again, was a pleasure!

nilsnolde commented 4 years ago

@danilopinotti what's your username on https://wordpress.org? Then I can include you in the contributors section.

danilopinotti commented 4 years ago

@danilopinotti what's your username on https://wordpress.org? Then I can include you in the contributors section.

@nilsnolde My username on wordpress.org is danilopinotti