FND / tiddlywebplugins.gitstore

Git-based text store for TiddlyWeb
1 stars 1 forks source link

Gitstore TW doesn't save to the right bag - text store does #8

Closed pmario closed 11 years ago

pmario commented 11 years ago

I did download the latest gitstore plugin with pip install -U ...

As it is a bit complicated to explain, so I did record a video that exactly shows what I did + the issure:

http://www.youtube.com/watch?v=akMaGdso82k - You can fast forward from 2:30 to ~3:50

FND commented 11 years ago

I'm afraid I find it very hard to follow a 4 min. video using convenience tools (your tweb-config, which is otherwise pretty cool) which obscure what's really going on.

...

Having watched the video again, I'm still not able to glean sufficient information from it. Can you provide a minimal test case similar to this: http://sandbox.tiddlyspace.com/gitstore%20instance

pmario commented 11 years ago

As I wrote, your sandbox instance doesn't have this problem, since the default initialisation of the client matches the default tiddlywebwiki instance. One has to create a new recipe, that doesn't contain the default stuff.

I did copy the whole instance directory to dropbox. http://dl.dropbox.com/u/28489257/gitstore/gs.tar.bz2

I did create the instance on the RasPi with:

sudo pip install -U tiddlywebwiki
sudo pip install -U tiddlywebplugins.gitstore
FND commented 11 years ago

As I wrote, your sandbox instance doesn't have this problem, since the default initialisation of the client matches the default tiddlywebwiki instance. One has to create a new recipe, that doesn't contain the default stuff.

I was asking for a minimal test case that describes the console commands necessary to reproduce the issue.

I don't see anything suspicious in that store - but in the absence of a proper description of the issue, I'm afraid I don't know what I should be looking for.

pmario commented 11 years ago
#!/bin/bash
cat <<EOT > _recipe_asdf_public
desc: Here comes the TW description TODO
policy: {"read": [], "create": [], "manage": ["pmario"], "accept": [], "write": [], "owner": "pmario", "delete": []}

/bags/system/tiddlers
/bags/asdf_public/tiddlers
EOT

twanager recipe asdf_public < _recipe_asdf_public

cat <<EOT > _bag_public
{"policy": {"read": [], "create": [], "manage": ["pmario"], "accept": [], "write": [], "owner": "pmario", "delete": []}, "desc": ""}
EOT
twanager bag asdf_public < _bag_public
FND commented 11 years ago

Thanks, this allowed me to reproduce this issue. (Could you just update your description to mention that tiddlers should be created via http://0.0.0.0:8080/recipes/asdf_public/tiddlers.wiki to ensure others can reproduce it too.)

It appears that the store contents are correct, but some reason TiddlyWebConfig is getting confused here:

var recipe = tiddler.fields["server.recipe"];
var workspace = recipe ? "recipes/" + recipe : "bags/common";

The former ends up as an empty string, so the latter ends up targeting the common bag.

The store's tiddler_get returns the right thing, yet in the TiddlyWiki serialization's _tiddler_as_div tiddler.recipe ends up as None.

@cdent: Any ideas what might be going on here?

cdent commented 11 years ago

Does the JSON rep for the individual tiddler and the recipe collection get the correct recipe attribute.

That is: is the problem narrowly within just the tiddlywiki serialization?

FND commented 11 years ago
$ curl http://0.0.0.0:8080/recipes/asdf_public/tiddlers.json | python -m json.tool | grep recipe
    "recipe": null,
    "recipe": null,
    "recipe": null,
    "recipe": null,
    "recipe": null,
    "recipe": null,

$ curl http://0.0.0.0:8080/recipes/asdf_public/tiddlers/TiddlyWebConfig.json | python -m json.tool | grep recipe
    "recipe": "asdf_public",

I'll be away until next weekend, so I'm afraid I won't get around to digging deeper until then.

cdent commented 11 years ago

In my own testing the default recipe also has null recipes as well, so the test can be much more minimal. I gotta go to do a mother's day thing but will look into it more later.

cdent commented 11 years ago

The problem is similar to 24ba9e1cadfe19fc03abb85dd169db7167d4b3f4, this is the fix:

diff --git a/tiddlywebplugins/gitstore/__init__.py b/tiddlywebplugins/gitstore/__init__.py
index 52e7dbc..500b447 100644
--- a/tiddlywebplugins/gitstore/__init__.py
+++ b/tiddlywebplugins/gitstore/__init__.py
@@ -219,6 +219,7 @@ class Store(TextStore):
         self.serializer.object = revision_tiddler
         self.serializer.from_string(tiddler_string)
         revision_tiddler.revision = tiddler.revision
+        revision_tiddler.recipe = tiddler.recipe
         return revision_tiddler

     def _commit(self, message, *filenames):

(Get revision is the pathway here because the tiddler has already been loaded once but is now being reloaded out of the Tiddlers collection)

As discussed before this indicates a fundamental conflict between the way the original stores operate and new stores want to operate. The old ones are side effecty and evidently this isn't made clear enough. However, it is part of the notion of how stores work:

The missing word in the discussion is probably "populate".

I recognize that this is annoying and messy if you haven't already bought into the style, but it is pretty unlikely that it will ever change at this point.

FND commented 11 years ago

Thanks @cdent - but it doesn't seem to make a difference: https://github.com/FND/tiddlywebplugins.gitstore/compare/recipe-tiddlers-fix The test there is failing; tiddler.recipe remains None.

It's quite possible that my test is flawed (going through control seems a bit ... mislayered?), but as explained above, I currently have limited resources to investigate.

cdent commented 11 years ago

That test has the expected result. The fix only comes into play during serialization. The control method returns empty tiddler, still awaiting population, no recipe set.

FND commented 11 years ago

So how do we fix it? (I'm happy to defer all this until next week.)

cdent commented 11 years ago

The fix I showed fixes it, I tested it in an instance.

Sent from my iPhone

On Mar 10, 2013, at 14:20, FND notifications@github.com wrote:

So how do we fix it? (I'm happy to defer all this until next week.)

— Reply to this email directly or view it on GitHub.

FND commented 11 years ago

Sorry, I was unclear: My "it" was referring to the test. I'd hate to fix a bug without also having a test for it.

cdent commented 11 years ago

Either:

Sent from my iPhone

On Mar 10, 2013, at 14:55, FND notifications@github.com wrote:

Sorry, I was unclear: My "it" was referring to the test. I'd hate to fix a bug without also having a test for it.

— Reply to this email directly or view it on GitHub.

FND commented 11 years ago

done and released (ed3a5f01bc64a7d40fbd06545da9b6a24fb1e349) - many thanks to both of you!

cdent commented 11 years ago

Just for sake or historical context or whatever, now that I'm back at my desk:

The cause a lot of empty tiddlers (just title and bag on the object) is optimization. The analysis that led to creation of the Tiddlers Collection concept showed there were a great deal of tiddlers being passed through the various stages of the system as populated tiddlers that either didn't need to be or only needed to be for a brief moment of their lifespan. The optimization significantly reduces both CPU and memory usage in the common cases while also providing a huge boost in efficiency on those system where a caching store is being used. The gist is that it is more efficient to get single tiddlers repeated times in a single request than it is to carry around a large collection of multiple fat tiddlers.

The cost is a fair amount of WTF but at the time this was considered okay: TiddlyWeb had passed beyond the stage in its lifecycle where it was supposed to be a reference and into the stage where it was providing lots of tiddlers to lots of people and minimizing the impact of request A on request B became most important.

Were there ever a redesign (extremely unlikely) to "build a tiddlyweb to do the service awesomely" then it's likely a few things would change: