search

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

Doesn't update ipynb when git is updated. #26

Open tirohia opened 3 years ago

tirohia commented 3 years ago

Not sure how this is meant to work.

Install of plugin, and WP pusher went fine. I created a new page, added shortcode for one of the examples,

[git-github-jupyter url="https://github.com/GIScience/openrouteservice-examples/blob/master/python/ortools_pubcrawl.ipynb"]

and it renders fine. If I then change it to one of my notebooks, it changes and renders the new book correctly. I've since made multiple updates over several hours to the file on github that the shortcode is referencing, but no changes show up in the rendered ipynb on wordpress.

Deleting the page, and adding the shortcode to a new page, does not fix this, so I'm guessing it's a caching issue of some sort? There is mention in #20 of static vs dynamic caching being implemented soon. As best I can tell, the suggested workaround for an insufficiently dynamic cache is to adjust the limit or cache_ttl in the config.json. Adjusting these made no change - I dropped limit to 1 and cache_ttl to 6, activated/deactivated the plugin and it's still looking at a version of the notebook that was initially loaded to github several hours ago.

Not sure what else I should be looking for or where to start looking to be able to diagnose/fix this?

Cheers Ben

nilsnolde commented 3 years ago

Probably you're experiencing this: https://github.com/gis-ops/wordpress-markdown-git#static-caching-cache_strategystatic

If you change the TTL by a second or so it should reload. If not, then your page is cached browser-side for some reason.

BTW, I'd recommend installing via WP plugin manager. Most of the time either method should be fine, but no guarantee that master is in a usable state.

nilsnolde commented 3 years ago

Ah sorry, you edited your question quite a bit.. Ok, I'll quickly have a try.

nilsnolde commented 3 years ago

I can confirm with a test notebook. Having a real quick look at the code I don't see right away the problem.. It's really strange, not even cache_strategy=dynamic reloads the notebook, but that should definitely invalidate the caching stuff.. It may take a while to get this sorted, I don't have time for it this week. If you want you can have a look too, it should be somewhere around this function:

https://github.com/gis-ops/wordpress-markdown-git/blob/eb8074c6fbe5a4ce7da05720a66458a1806d8e0f/documents-git/includes/providers/class-base-loader.php#L82

tirohia commented 3 years ago

Ah, yes, I did sorry. I found the references to the caching a few minutes after I posted the issue.

The good news is that it appears to have updated overnight, which means that it does tick over at some point. Assuming that it ticking over wasn't a result of me breaking it and putting everything back last night.

The bad news is that my PHP is sufficiently rusty (it's been at least a decade) that I haven't been able to be able to figure out what's going on in that function. I can reliably break it, but have not yet make any sense of the breakages.

As an aside, I install a debugging plugin, and when loading the page I get a bunch of errors saying something like:

  wp_enqueue_style was called incorrectly. Scripts and styles should not be registered or enqueued until the wp_enqueue_scripts, admin_enqueue_scripts, or login_enqueue_scripts hooks. This notice was triggered by the markdown_git handle. Please see Debugging in WordPress for more information. (This message was added in version 3.3.0.)

It's coming out of three places, markdown_git, github_markdown and nbconvert_git handles, from documents-git.php. Unfortunately I am insufficiently au fait with the internals of wordpress, the plugin and php to be able to figure out if that is relevant at all. Will keep looking, don't know how useful I'll be though.

nilsnolde commented 3 years ago

Cool, thanks for the warning, wasn't aware of that. I'll see if I can change that.

Usually I debug with WP docker & PHPStorm, but the setup is fairly involved if you want breakpoint support. Always a nightmare when I setup a new computer and didn't check in the project config.

Anyways, very strange (if good..) that it updated overnight! The default for TTL is 1 hour.. I really want the dynamic cache with web hooks (also fault-affine, but so elegant!). I'll carve out a few hours soon to check this and implement dynamic cache.

nilsnolde commented 3 years ago

Finally had some time to look into this and I can't reproduce (anymore).. I'll publish a new version soon just to be safe (tested dev setup).

nilsnolde commented 3 years ago

Sorry, I actually could reproduce it. And the problem is nbviewer which renders the notebook (and the only reason we can support it): https://github.com/jupyter/nbviewer/issues/914. They are supposed to have a flush_cache option in the query parameters, but neither true nor false works.. I'll leave it true, might be they fix it at some point.

That also explains why it was all of a sudden "just updated" for you @tirohia . Sorry to not have better news..