comments plugin / appspot tiddlywebwiki sandbox
Oveek
TiddlyWebWiki theme for our intranet. This has been sort of hindering
my progress for a while. I've been wanting to create an easy to read,
and fresh look, but there are so many page elements it's a bit painful
tweaking everything.
Started off with the nice theme on LewcidTW and after many iterations,
got something nice together.
Most recently I started fooling with the look of the comments
generated by the CommentsPlugin. I was initially trying to make
changes geared towards streamlining the appearance of the whole
comment interface and cutting down a bit on the visual noise. Then I
came across the problem of replies to comments mentioned on peermore.
I think I found a fix for that problem, and also made a few other
changes:
* Fixed replies to comments problem
* Created a different comment naming scheme.
* Changed style so comment input box is initially hidden and can be
toggled hidden/shown using a button.
* Moved the 'reply to this comment' link into the comment header.
- I believe I've zeroed in on the source of the replies to comments
problem. It has a really simple fix, but the underlying cause is a
little bit subtle. The cause is rooted in a global object being copied
by reference.
Firstly, the symptom I observed is that when a reply is made to
another comment (and the page is not reloaded between making the
comment and its reply) the fields of the parent are overwritten with
the fields of the reply. This causes the parent's 'daddy' and possibly
'prev' fields to incorrectly point to itself.
The offending code is in cmacro.saveTiddler(). The original is:
store.saveTiddler(tiddler.title, tiddler.title, tiddler.text,
tiddler.modifier, tiddler.modified, tiddler.tags, merge
(config.defaultCustomFields, tiddler.fields), false, tiddler.created)
},
The problem is caused by merge(config.defaultCustomFields,
tiddler.fields). The function call copies tiddler.fields into
config.defaultCustomFields instead of the opposite, which is probably
more in line with the logic of what is intended (namely merge
defaultCustomFields into the fields of the new comment tiddler.)
Swapping the arguments to merge() solves the problem:
store.saveTiddler(tiddler.title, tiddler.title, tiddler.text,
tiddler.modifier, tiddler.modified, tiddler.tags, merge
(tiddler.fields, config.defaultCustomFields), false, tiddler.created)
Theory on what's happening: In most cases the order of arguments to
merge() doesn't matter too much, but it does here.
* The first thing is that config.defaultCustomFields is a global
object and persists through the life of an open TiddlyWiki.
* The second point is that when a tiddler is saved its attributes get
assigned by reference in Tiddler.prototype.assign().
The result is that the field attribute of consecutive comments end up
pointing to the same values / variable / memory location.
When a parent comment is created, merge() copies tiddlers.fields into
config.defaultCustomFields. Then store.saveTiddler saves, causing the
parent comment's field attribute to become a pointer to
config.defaultCustomFields.
Next when a reply to the parent (or any other comment) is made, merge
is called again, and tiddler.fields of the reply is copied to
config.defaultCustomFields--overwriting the previous values. After the
call to store.saveTiddler, the new reply comment's field attribute
also points to config.defaultCustomFields. At that point the field
attribute of both parent and reply point to the same memory location.
If you look at the fields of both they will show the same values (the
values of the new reply comment).
Now when the new reply comment is saved back to the server, the parent
is dirty since its fields changed, and the parent gets saved too with
the same field values as the reply...and that's how the fields become
messed up.
- The other stuff is pretty much take it or leave it. I tried out a
new comment naming scheme so you can look at the timeline and know
what tiddler a comment was made on.
There are a couple problems with this scheme, long names being one.
Some variation of this would might be better. The main motivation was
to let users know what tiddler was commented on just by seeing the
comment title.
You can see the ComentPlugin and style stuff here:
http://omcode.appspot.com/recipes/default/tiddlers.wiki
That's a 'sandbox' I put up on appspot to share TiddlyWebWiki stuff
with everyone. By the way, I agree with a lot of Jeremy's comments in
the other thread, and it would be great to have a TiddlyWebWikiSpot
sometime. I know it's somewhere in the pipeline. I think it will give
people an 'easy in' for playing with TiddlyWeb.
...I'm using the latest git master on appspot, and found a problem
while setting it up. There's currently an ETag problem that occurs
when updating existing tiddlers. The issue results from mismatched
revisions.
At present the googledata store doesn't use revisions, but the
revisions field is still set (at various times) on both the client
side and server side. The problem is the revision field is sometimes
set to 1 and sometimes to 0 depending on whether you're creating,
modifying, or deleting tiddlers.
The relevant server side stuff is in tiddlyweb/web/handler/
tiddler.py.
The client side stuff is in the TiddlyWebAdaptor plugin.
This can be sorted out by always setting the revisions to 1 or always
setting them to 0. Either one would work, but each option requires
modifications in different places (functions). I picked setting the
revisions to 0 because I think it's the easier way (existing tiddlers
retrieved from the googledata store default to a revision number of
0).
server side:
There are a couple lines in _put_tiddler in tiddlyweb/web/handler/
tiddler.py:
try:
revision = store.list_tiddler_revisions(tiddler)[0]
except StoreMethodNotImplemented:
revision = 1
tiddler.revision = revision
Since the googledata store does not implement revisions, the above
line sets tiddler.revision to 1 before the ETag check between
incoming_etag and tiddler_etag. But the tiddlers retrieved from the
datastore (when a tiddlywiki is loaded) have revisions default to 0,
so when attempting to update an existing tiddler, the etags don't
match: the revision on the incoming tiddler is 0 and the revision on
the tiddler in the store is set to 1 by the line above.
To fix this I just changed the above except clause:
except StoreMethodNotImplemented:
revision = 0
client side:
The change on the server side requires a change to TiddlyWebAdaptor.
The adaptor.generateETag() function of the TiddlyWebAdaptor sets
revisions on *new* tiddlers to 1:
if(typeof revision == "undefined") {
revision = 1;
}
To make the adaptor match with the modified tiddler.py, I changed the
above to:
if(typeof revision == "undefined") {
revision = 0;
}
I don't know if these two changes break anything else, but they fix
the etag conflicts and allow to create, save, and delete in the
googldata store. I don't know if there's a less invasive way to fix
this, but the change to tiddler.py seems unavoidable.
Okay signing off for now. Nice to see the activity in the new group.
Chris Dent
Zow, fabulous post. Lots of great stuff.
CommentsPlugin author mahemoff is on the list so perhaps he can use
this as a guide to fix that stuff.
Other comments from me within.
On Jul 23, 2009, at 2:32 PM, Oveek wrote:
> Been working on some cosmetic stuff, trying to put together a
> TiddlyWebWiki theme for our intranet. This has been sort of hindering
> my progress for a while. I've been wanting to create an easy to read,
> and fresh look, but there are so many page elements it's a bit painful
> tweaking everything.
I'm glad to hear this. One of the things that always falls off my list
of priorities is making stuff easy to read, fresh, etc.
> Theory on what's happening: In most cases the order of arguments to
> merge() doesn't matter too much, but it does here.
>
> * The first thing is that config.defaultCustomFields is a global
> object and persists through the life of an open TiddlyWiki.
> * The second point is that when a tiddler is saved its attributes get
> assigned by reference in Tiddler.prototype.assign().
>
> The result is that the field attribute of consecutive comments end up
> pointing to the same values / variable / memory location.
Good catch.
> - The other stuff is pretty much take it or leave it. I tried out a
> new comment naming scheme so you can look at the timeline and know
> what tiddler a comment was made on.
I struggle with the wiki notion of referring to anything by its name,
rather than some more persistent ID. It's a great and powerful thing,
up until the point when it is not...and then it's really not. So, for
example what happens to comments when the root tiddler is renamed.
> There are a couple problems with this scheme, long names being one.
> Some variation of this would might be better. The main motivation was
> to let users know what tiddler was commented on just by seeing the
> comment title.
That is valuable. On http://tiddlyweb.peermore.com/ the Timeline is
filled with comments that convey no meaning, just that lots of
comments have happened, and while you can click on the comment to see
its breadcrumb, because of the way navigation works in tiddlyweb, you
can scroll the timeline out of view when you click, so opening up all
the comments in quick succession to have a browse is not easy.
I think we're going to find that a lot of the TiddlyWiki UI aspects
that work great when there is only one author start to go a bit sour
when there are many.
> That's a 'sandbox' I put up on appspot to share TiddlyWebWiki stuff
> with everyone. By the way, I agree with a lot of Jeremy's comments in
> the other thread, and it would be great to have a TiddlyWebWikiSpot
> sometime. I know it's somewhere in the pipeline. I think it will give
> people an 'easy in' for playing with TiddlyWeb.
It should also be fairly straightforward to make it awesome.
Did you have [m]any issues with getting things running on GAE? I
haven't tried there lately; don't keep tiddlyweb.appspot.com up to date.
> ...I'm using the latest git master on appspot, and found a problem
> while setting it up. There's currently an ETag problem that occurs
> when updating existing tiddlers. The issue results from mismatched
> revisions.
Ah, yes. This is somewhat expected. Or rather doesn't surprise me. The
revision handling in ETags with non-revision-using stores is entirely
untested. If you can delineate some English simple tests that sort of
cover the problems you've encountered I'll add them as code to the
test suite.
> except StoreMethodNotImplemented:
> revision = 0
This seems like it is probably correct, it also points out that there
is some duplication in tiddler_put in the text store, which looks up
the revision on disk a second time (after the handler has already done
it). Either only the store should do it, or only calling code, not both.
> I don't know if these two changes break anything else, but they fix
> the etag conflicts and allow to create, save, and delete in the
> googldata store. I don't know if there's a less invasive way to fix
> this, but the change to tiddler.py seems unavoidable.
Have you had an opportunity to see if your changes continue to work
with a revision supporting store, such as the text store of sql store?
The thing I'm most unsure about is what the right fix is in the client
code. There it seems like the revision should always be 1 for a newly
created tiddler.
FND
Indeed - thanks for your efforts!
> CommentsPlugin author mahemoff is on the list so perhaps he can use
> this as a guide to fix that stuff.
Mike is aware of your work and will likely respond soon.
> The thing I'm most unsure about is what the right fix is in the client
> code. There it seems like the revision should always be 1 for a newly
> created tiddler.
I agree. Not sure what the best way to resolve this is either.
The easy way out would be to say that TiddlyWebWiki expects a
revision-supporting store by default, with the option of having a
different TiddlyWebConfig set the base revision to 0 if required
(perhaps store properties* could be reported by the status plugin).
-- F.
* it gets more complicated with the multistore...
Michael Mahemoff
> > Zow, fabulous post. Lots of great stuff.
>
> Indeed - thanks for your efforts!
>
such depth. I'll need to take some time to dive into the code with all
this in mind and hopefully we can get a fix in ASAP.
Oveek
> I'll need to take some time to dive into the code with all
> this in mind and hopefully we can get a fix in ASAP.
to comments feature which the more faint of heart (such as myself)
probably wouldn't have attempted.
To possibly save you some time, the only real thing of consequence
about the CommentsPlugin from my post was about swapping the arguments
to merge(). That's the fix that worked for me.
> That is valuable. Onhttp://tiddlyweb.peermore.com/the Timeline is
> filled with comments that convey no meaning...
The timeline on peermore is pretty much what motivated me, but...
> So, for example what happens to comments when the root tiddler is renamed.
I've got some partially conceived solutions floating around and I
think a workaround could emerge.
One option would be to have a serverside plugin rename comments when a
root tiddler is renamed, but I think there may be a more elegant way
around this that doesn't require renaming batches of comment tiddlers.
An idea I have that's still fuzzy is having comments display dynamic
titles. The main points would be:
* Have a constant (across tiddler renames) field on the root tiddler
that is used to link comments to the root tiddler.
* Use a custom ViewTemplate for comments that replaces the default
title macro with a macro that looks up the root tiddler's title and
uses that to construct the comment name.
The basic idea being to have one constant name which is used to save
the comment tiddler in the store, and another name which is actually
displayed in the UI, and can optionally be generated dynamically. Of
course this needs to apply to all the places tiddler names apply
including tiddler titles, and tiddlylinks in tiddler bodies and the
timeline. Maybe somehow using aliases? I haven't given much thought to
implementation details yet, just throwing the idea out there for now.
> Did you have [m]any issues with getting things running on GAE? I
> haven't tried there lately; don't keep tiddlyweb.appspot.com up to date.
box.
How hard would it be to add support for roles to the store? The
sandbox is a little vulnerable right now without any access control.
Chris Dent
On Jul 24, 2009, at 10:42 PM, FND wrote:
> I agree. Not sure what the best way to resolve this is either.
> The easy way out would be to say that TiddlyWebWiki expects a
> revision-supporting store by default, with the option of having a
> different TiddlyWebConfig set the base revision to 0 if required
> (perhaps store properties* could be reported by the status plugin).
Not keen on this: it doesn't resolve the problem, it just moves it
around. Having to query the server for store properties is like a big
design slap saying, "you've got a big problem with the genericity of
your system". So lets see if we can fix it for real.
I think the issue is the server side comprehension or handling of the
incoming tiddler. It should ignore the revision field of the incoming
tiddler when it is a strict PUT (i.e. not a POST of tiddler revision
history as done with a rename). If the revision field is set to None
before entering the store things should "just work".
This works out logically because we have already made a statement in
the design of the system that revisions, when they exist, are
immutable. Whenever we PUT a tiddler we are creating a new revision,
and it is up to the store to determine what the next revision id is.
I need to go back through Oveek's messages to understand the Etag
situation a bit better, I think there may be a logic flaw there, as
that's really the crux of the biscuit. My brain is not currently
warmed up enough...
Chris Dent
On Jul 27, 2009, at 7:07 AM, Oveek wrote:
> * Have a constant (across tiddler renames) field on the root tiddler
> that is used to link comments to the root tiddler.
In this context is the root tiddler, the tiddler which is being
commented upon? If so, that might be a non-starter. One of the
advantages of the CommentsPlugin as it stands right now is that it
works entirely from client code and requires no write permission on
the tiddlers being commented upon, just create permission in the
comment bag.
>> Did you have [m]any issues with getting things running on GAE? I
>> haven't tried there lately; don't keep tiddlyweb.appspot.com up to
>> date.
>
> Aside from the revision conflict everything worked right out of the
> box.
Remarkable! :)
> How hard would it be to add support for roles to the store? The
> sandbox is a little vulnerable right now without any access control.
Not too hard, but mildly complex:
* A User model would need to be added to the googledata store
* An interface of some sort that allowed for the assignment of roles
to google user names
* Extending the extractor so it looked up the current google user in
the User model to get their roles (if any)
chris...@gmail.com
On Jul 27, 12:44 pm, Chris Dent <chris.d...@gmail.com> wrote:
> I think the issue is the server side comprehension or handling of the
> incoming tiddler. It should ignore the revision field of the incoming
> tiddler when it is a strict PUT (i.e. not a POST of tiddler revision
> history as done with a rename). If the revision field is set to None
> before entering the store things should "just work".
>
> This works out logically because we have already made a statement in
> the design of the system that revisions, when they exist, are
> immutable. Whenever we PUT a tiddler we are creating a new revision,
> and it is up to the store to determine what the next revision id is.
Can you guys confirm or deny that the revisions1 branch of tiddlyweb
on github
http://github.com/tiddlyweb/tiddlyweb/tree/revisions1
fixes, breaks or changes the behavior of PUT revisions?
I think it is a little more consistent, but I'm not certain it results
in the right behavior.
Michael Mahemoff
into it - your theory was spot on about this being the cause for the
bug.
> * Created a different comment naming scheme.
changed the title to include the name of the root tiddler - so it's of
the format cakeComment98765 (where "cake" is the tiddler being
commented on and "98765" is a GUID). I'd like to change the GUID
component to reflect the position in the hierarchy, e.g. 1_1_3, but
it's not trivial as there's the possibility of a conflict in the case
of a TiddlyWeb-backed comment store (as opposed to a standalone
TiddlyWiki). i.e. commenter A submits a new comment, commenter B
already has the page open and submits a new comment - the client will
be trying to submit it with an ID that's already being used. The way
around this is to introduce a callback handler, which I think would
work, but needs time to implement.
> * Changed style so comment input box is initially hidden and can be
> toggled hidden/shown using a button.
an option. I designed it to be visible as I feel it helps the user to
know comments are welcome, as well as making the means of commenting
clearer. However, I realise not all interfaces have the same
requirements and it would be worth considering making this an option.
(Incidentally, the always-visible textarea is something I felt was
worth fighting for; its presence means the code cannot be fully
recursive. If the textarea was hidden - as they are with replies - the
comments UI algorithms would be somewhat simpler because there would
be no need to treat the top-level comments as a special case.)
Thanks again for your contribution.
Chris Dent
On Jul 27, 2009, at 6:52 PM, Michael Mahemoff wrote:
> Yes, this is certainly something that needs work. For now, I've
> changed the title to include the name of the root tiddler - so it's of
> the format cakeComment98765 (where "cake" is the tiddler being
> commented on and "98765" is a GUID). I'd like to change the GUID
> component to reflect the position in the hierarchy, e.g. 1_1_3, but
> it's not trivial as there's the possibility of a conflict in the case
> of a TiddlyWeb-backed comment store (as opposed to a standalone
> TiddlyWiki). i.e. commenter A submits a new comment, commenter B
> already has the page open and submits a new comment - the client will
> be trying to submit it with an ID that's already being used. The way
> around this is to introduce a callback handler, which I think would
> work, but needs time to implement.
Encoding structuring information into the comment title seems like a
bad idea to me. In addition to clouding the presentation with extra
stuff, you're making the identifier of the comment have meaning, which
almost always leads to trouble down the road. Better, I think, to use
tiddler fields in the way you've mostly done it already making the
tree of comments traversable solely by data on the tiddlers themselves.
FND
>> I'd like to change the GUID component to reflect the position in
>
> Encoding structuring information into the comment title seems like a
> bad idea to me.
I tend to agree with Chris on this.
While it's probably worth having the root tiddler's title in the
comment's title (mostly due to the timeline situation*), I don't think
the hierarchy-dependent ID conveys enough useful information to make it
worth the trouble.
-- F.
* while I can see Oveek's point about separating ID and caption, it's a
can of worms that would require patching all over the place, including
third-party plugins and TiddlyWeb serializers
Michael Mahemoff
> * while I can see Oveek's point about separating ID and caption, it's a
> can of worms that would require patching all over the place, including
> third-party plugins and TiddlyWeb serializers
happen at some stage; in an app like TiddlyDocs, with multiple "word
processing" type documents in the same instance, you can't force users
to use a different title for every section.
FND
>> can of worms that would require patching all over the place, including
>> third-party plugins and TiddlyWeb serializers
>
> In some "TiddlyCMS" apps, something like that is going to have to
> happen at some stage
Absolutely.
My Cork experiment, for example, uses (pseudo-)UUIDs and a caption field.
However, retrofitting TiddlyWiki-based verticals that way would be hard.
(I realize TiddlyDocs is TiddlyWiki-based, but it's probably fairly
self-contained.)
-- F.
Oveek
comments plugin.
> it's not trivial as there's the possibility of a conflict in the case
> of a TiddlyWeb-backed comment store (as opposed to a standalone
> TiddlyWiki). i.e. commenter A submits a new comment, commenter B
> already has the page open and submits a new comment - the client will
> be trying to submit it with an ID that's already being used.
that's been on my mind about handling edit contention. In the scenario
you describe, what happens when person A and B are editing the same
tiddler? Just like with the comments, If B submits their change after
A, then A's changes to the tiddler get clobbered by B. I think
revisions will play in a role in resolving conflicts, but getting back
to comment names...
> I tend to agree with Chris on this.
> While it's probably worth having the root tiddler's title in the
> comment's title (mostly due to the timeline situation*), I don't think
> the hierarchy-dependent ID conveys enough useful information to make it
> worth the trouble.
I'm tending to agree that hiearachy information in the title is of
questionable value. Once you get a couple levels deep (replies to
replies to replies...) deciphering the hierarchy id becomes non
trivial and frankly doesn't look very good. On tiddlyCube (the
sandbox) I'm trying a scheme that uses hierarchy info and there's a
comment with the title:
comment 1ba on Comments demo
The '1ba' part means the comment is the first reply to the second
reply to comment1. This is still within the realm of being acceptable,
but past this and it starts getting a little ridiculous.
As an aside, the word 'comment' is pretty long and almost feels
redundant, but then you don't know a comment is a comment without it.
As a reader the main piece of info worth having is what was the
subject of the comment, namely what tiddler was commented on? Of
course it's still necessary for tiddlers to have unique IDs so
something needs to separately ID the comments.
My preference is that something, whatever it is, be something human
readable and not something mysterious to the user.
Then there's also the matter of the root tiddler being renamed.
On Jul 29, 3:14 am, FND <F...@gmx.net> wrote:
> >> * while I can see Oveek's point about separating ID and caption, it's a
> >> can of worms that would require patching all over the place, including
> >> third-party plugins and TiddlyWeb serializers
>
> > In some "TiddlyCMS" apps, something like that is going to have to
> > happen at some stage
meaningful comment names is too. Hopefully we can work out some
solutions on these fronts.
FND
> I think revisions will play in a role in resolving conflicts
Normally, the ETag handling should prevent any clobbering and report an
edit conflict instead.
Having said that...
a) I don't know how well this works for revision-less stores (that's the
topic of the other branch of this thread, I suppose)
b) not sure whether Mike's CommentsPlugin supports ETags (presumably, it
just relies on the TiddlyWebAdaptor - at least in the TiddlyWiki
version)
c) there's currently a bug in the TiddlyWebAdaptor which affects
newly-created tiddlers:
http://trac.tiddlywiki.org/ticket/1112
> My preference [for comments] is that something, whatever it is, be
> something human readable and not something mysterious to the user.
I agree, there should be at least a hint somewhere. So some combination
of title + UUID seems like a good compromise - even at the risk of
renaming messing things up (which, in fact, probably is the case even
now when we're just using custom fields).
In fact, retaining the original tiddler title in the comment's title
could be considered the Right Way, as that was the context at the time.
-- F.
Michael Mahemoff
> just relies on the TiddlyWebAdaptor - at least in the TiddlyWiki
> version)
happen since you're appending a tiddler identified by a GUID.
> I agree, there should be at least a hint somewhere. So some combination
> of title + UUID seems like a good compromise - even at the risk of
> renaming messing things up (which, in fact, probably is the case even
> now when we're just using custom fields).
> In fact, retaining the original tiddler title in the comment's title
> could be considered the Right Way, as that was the context at the time.
the comment is titled, which would be interpolated. e.g. you could say
title:"{rootTitle}Comment{guid}".
chris...@gmail.com
>
> Can you guys confirm or deny that the revisions1 branch of tiddlyweb
> on github
>
> http://github.com/tiddlyweb/tiddlyweb/tree/revisions1
>
> fixes, breaks or changes the behavior of PUT revisions?
>
> I think it is a little more consistent, but I'm not certain it results
> in the right behavior.
profiling and pedagogical purposes, but I can see other opportunities
stemming from its use) and I'm running into some revision handling
issues. If you guys have a had chance to look at this stuff and could
give a "this fixes it" or "this helps but is not enough" or "blows up"
that would be useful.
From what I'm learning right now, it may be that the store's contract
for tiddler_put needs to be a bit more explicit about what it is going
to do with the tiddler object that has come in. tiddler.revision needs
to be managed carefully so etags are created properly.
FND
> give a "this fixes it" or "this helps but is not enough" or "blows up"
> that would be useful.
I've reviewed the revisions1 branch again (which is now just two
seemingly simple changes now) - but I'm not entirely confident
predicting whether it's correct or not.
Having said that, it seems to be doing the right thing, and testing it
against the text store revealed no issues.
Oveek, did you get a chance to test it against your revisionless store?
-- F.
Oveek
> Oveek, did you get a chance to test it against your revisionless store?
googledata store and I'm having some etag conflicts. I initially
reported some inconsistencies in the revision handling, and I still
think that's the case. At the time I hadn't really pinned down what
was going on, but I think I have a better idea of what's happening
now. Still following up a couple loose ends; I'll check back shortly.
Chris Dent
On Aug 6, 2009, at 2:25 PM, Oveek wrote:
> I've done some testing with the revisions1 branch against the
> googledata store and I'm having some etag conflicts. I initially
> reported some inconsistencies in the revision handling, and I still
> think that's the case. At the time I hadn't really pinned down what
> was going on, but I think I have a better idea of what's happening
> now. Still following up a couple loose ends; I'll check back shortly.
To add to the stack of available data about revisions handling I did a
little experimenting with the devtext store, which does not do
revisions. This is from the main code (release 0.9.55 in this case),
just to see what I could reproduce. I didn't really have any problems
with editing the same tiddler.
Also, under normal circumstances renaming a tiddler worked just fine,
as long as I was _only_ renaming. If I renamed a tiddler and also
edited its text, then I got an "Error saving f: OK" (f is the name of
the tiddler). Checking the tiddlyweb.log, no request is ever made to
the server, the error is purely on TiddlyWiki's side. If I then edit
the same tiddler, or any other tiddler, the tiddler is saved without
issue in the next sync back to server.
So I then confirmed the same issue was present with a store that does
revisions, and it is.
I'm using, as far as I know, the most recent TiddlyWebAdaptor etc, as
I'm working from a freshly created tiddlywebwiki instance.
So whatever issues might be present on the server side with revision
handling, there are also some client side issues that need to be
cleared up as well...
Oveek
have any problems.
One difference I see between running devtext and googledata is in the
server.page.revision field. Under devtext, tiddlers have
server.page.revision = 1 while under googledata, tiddlers have
server.page.revision = 0.
I'm not sure where it's getting set to zero, but under googledata
having tiddlers with server.page.revision = 0 is causing an edit
conflict when editing and saving an existing tiddler.
Oveek
get
too sidetracked
> revisions, and it is.
problems. Renaming works when a tiddler's title is changed or when a
tiddler's title and text are both changed.
I did hit another renaming problem though. I can't rename tiddlers
that have been commented on. I get this error in Firebug:
Unable to convert object of type 'function' to json.
As an offshoot of recent testing with the sqlstore I've temporarily
switched to an SQLite database. There are still some problems using
the sqlstore with postgres as the backend. Things are fine initially,
but problems crop up after using tiddlyweb for a while. There are
transactions that aren't completing cleanly. After a while an
increasing number of postgres listeners get stuck in "Idle in
transaction" status. This leads to an increased chance that subsequent
tiddlyweb operations raise an exception because of problems with
transactions that need to be rolled back, or other session related
problems.
For the record I've been troubleshooting the revision problems using:
* tiddlyweb revisions1 branch
* tiddlyweb-plugins master branch plus FND's plugins
* The latest ServerSideSavingPlugin, TiddlyWebConfig, and
TiddlyWebAdaptor from svn.
So to summarize a few of the issues I'm puzzling about:
* googledata store: Why do tiddlers have server.page.revision = 0
when they are loaded from the googledata store? This is causing etag
conflicts on tiddler edits. The devtext store, another revisonless
store, doesn't seem to have this problem, why?
* another general etag issue: the TiddlyWebAdaptor does not generate
etags unless a tiddler's workspace.type == "bag" (there's a comment
alluding to this in adaptor.generateEtag()). However, newly created
tiddlers have a workspace.type of "recipe". This means edits of newly
created tiddlers never undergo an etag check on the serverside, or
more accurately, get an automatic pass because incoming_etag =
"None" (you can verify this, and I can elaborate further later).
* sqlstore with postgres backend: Session handling / which
transactions are not completing cleanly?
* Why unable to rename tiddlers that have been commented on?
Chris Dent
On Aug 7, 2009, at 5:27 AM, Oveek wrote:
> I just tested revisions1 against the devtext store and also didn't
> have any problems.
Sigh. It looks like I had yet more unpushed code on one of my
machines. Yesterday it was in the tiddlywebwiki repo, today it was the
tiddlyweb revisions1 branch. This is what I get for switching to using
multiple machines.
In any case there's a chance our testing has been slightly different
as you revisions1 branch and my revisions1 branch are somewhat
different. What I've done is gone ahead and merged revisions1 to
master and pushed them both. If you could continue your testing from
the master branch we'll be on the same page.
I've just recreated my local app engine environment (c.f. above about
new machines) so can play long at home. I've just replicated this:
> One difference I see between running devtext and googledata is in the
> server.page.revision field. Under devtext, tiddlers have
> server.page.revision = 1 while under googledata, tiddlers have
> server.page.revision = 0.
And indeed that's not the desired behavior. I'll see where the fix
needs to happen.
Chris Dent
On Aug 7, 2009, at 5:27 AM, Oveek wrote:
> One difference I see between running devtext and googledata is in the
> server.page.revision field. Under devtext, tiddlers have
> server.page.revision = 1 while under googledata, tiddlers have
> server.page.revision = 0.
The proximal cause for this behavior is just simply that googledata
doesn't set the revision whereas devtext does, despite not supporting
revisions. This is because it subclasses the text store, and in code
in the super class revision does get set.
The following diff explicitly set the revision to 1. Making the store
responsible for the entire form of the tiddler is in line with how the
storage adaptors are theoretically supposed to work, but I'm not sure
this is the most elegant solution:
diff --git a/googleappengine/googledata.py b/googleappengine/
googledata.py
index 3d84cb2..40bd09b 100644
--- a/googleappengine/googledata.py
+++ b/googleappengine/googledata.py
@@ -152,6 +152,9 @@ class Store(StorageInterface):
if tiddler.type and tiddler.type != 'None':
tiddler.text = b64decode(tiddler.text.lstrip().rstrip())
+ # explicitly set the tiddler revision to 1 since we don't
support
+ # revisions.
+ tiddler.revision = 1
return tiddler
def tiddler_put(self, tiddler):
Chris Dent
On Aug 7, 2009, at 10:24 AM, Oveek wrote:
> So to summarize a few of the issues I'm puzzling about:
/me loves summaries
> * googledata store: Why do tiddler's have server.page.revision = 0
> when they are loaded from the googledata store? This is causing etag
> conflicts on tiddler edits. The devtext store, another revisonless
> store, doesn't seem to have this problem, why?
I think I've provided a moderately satisfactory fix for this in my
previous message. I'll go ahead and commit it.
> * another general etag issue: the TiddlyWebAdaptor does not generate
> etags unless a tiddler's workspace.type == "bag" (there's a comment
> alluding to this in adaptor.generateEtag()). However, newly created
> tiddlers have a workspace.type of "recipe". This means edits of newly
> created tiddlers never undergo an etag check on the serverside, or
> more accurately get an automatic pass because incoming_etag =
> "None" (you can verify this, and I can elaborate later).
FND, any comments?
> * sqlstore with postgres backend: Session handling / which
> transactions are not completing cleanly?
I'll look into this some. I suspect what's happening is that
exceptional conditions, such a tiddlers not being found and what not,
are raising exceptions that exit the core of the code before the
transactions are every resolved. I'll need to do some digging in the
sqlAlchemy docs to see the best way to handle this sort of thing.
> * Why unable to rename tiddlers that have been commented on?
Mike?
chris...@gmail.com
* One is to set 'autocommit' to 'True' when configuring the Session
(sql.py:238). This makes the system non-transactional.
* The other is before every raise No{Tiddler,Bag,Recipe} inject a
self.session.close().
Both of these are guesses. I don't have easy access to Postgres at the
moment.
Michael Mahemoff
>
root tiddler (that being commented on) by its title. This is an
intrinsic aspect of TiddlyWiki's design and not something comments
plugin can easily overcome. In the first instance, though, I will add
some checks so it does not cause any error when the root tiddler is
renamed.
It might be argued that's the desirable behaviour; that comments
shouldn't automatically be moved upon rename. That said, I can see it
would certainly be a useful feature, as an option.
I could possibly build a macro that renames a tiddler and updates
comments, all in one. Or - less use in this case but more widely
applicable - I could imagine a generic TiddlyWiki admin macro that
presents you with a form containing "field", "old name", "new name".
Then it would run through *all* tiddlers and effectively do a search-
and-replace.
FND
Ditto - thanks for that, Oveek!
Certainly helps tying up loose ends in this fairly complex thread.
>> * another general etag issue: the TiddlyWebAdaptor does not generate
>> etags unless a tiddler's workspace.type == "bag" [...]
>
> FND, any comments?
Yeah, I suspected this would come up sooner or later; back when that
adaptor was written, we didn't have enough solid use cases yet to decide
what should happen. Seems the situation is different now, which is good.
#1112* is already on my list for next week, so hopefully we can fix this
soon.
-- F.
Oveek
> Having a look at this it seems there are two options:
> * One is to set 'autocommit' to 'True' when configuring the Session
> (sql.py:238). This makes the system non-transactional.
> * The other is before every raise No{Tiddler,Bag,Recipe} inject a
> self.session.close().
> Both of these are guesses. I don't have easy access to Postgres at the
> moment.
look at this alternative which is more along the lines of the second
option:
There's a thread on the SQLAlchemy Google group about sessions, and
there's an excellent post in that thread by a fellow named Michael
Bayer, describing what looks to be a really nice way to deal with
session stuff.
The basic idea is to, in his words, "frame" session operations using a
Python decorator that handles opening sessions, and does rollbacks and
commits where necessary.
The thread is here: http://groups.google.com/group/sqlalchemy/browse_thread/thread/dc9d1b...
The specific post I'm talking about is 4th in the thread:
http://groups.google.com/group/sqlalchemy/msg/31982b6ccc2151aa
I think the approach looks quite clean and solid. Using the decorator
centralizes the session handling in a single function, and in the
given example also handles the case of nested transactions.
The example defines a decorator called @with_session(), which wraps
all methods that do session stuff. So in our case we would move the
self.session.commit() calls out of the methods and into the decorator.
The example decorator also does rollbacks when there are exceptions--
something we may need as well.
Anyway take a look at that post and see what you think, it's quite
illuminating and provides another potential approach that would let us
keep using transactions (as
opposed to using autocommit).
Chris Dent
On Aug 8, 2009, at 3:18 AM, Oveek wrote:
> I think the approach looks quite clean and solid. Using the decorator
> centralizes the session handling in a single function, and in the
> given example also handles the case of nested transactions.
It does look good. Using a decorator is a smart way to handle this
sort of stuff.
Where TiddlyWeb differs is that (from the posting) this:
The common example is a web framework where the above framing
takes place in the BaseController class.
doesn't really apply in the adaptation style of persistence that
TiddlyWeb does. The gap between model and controller is
(intentionally) far wider than it is in the usual web framework.
I don't think that presents a problem in this case, it just moves the
locus of the decoration somewhat.
I think what can be done is that the decorator goes in sql.py and
their public methods get decorated.
> The example defines a decorator called @with_session(), which wraps
> all methods that do session stuff. So in our case we would move the
> self.session.commit() calls out of the methods and into the decorator.
> The example decorator also does rollbacks when there are exceptions--
> something we may need as well.
In the example decorator all exceptions are caught and the rollback
happens. We'll need to reraise so that things like NoBagError make it
upstream.
> Anyway take a look at that post and see what you think, it's quite
> illuminating and provides another potential approach that would let us
> keep using transactions (as
> opposed to using autocommit).
Is a good find.
Oveek
yesterday it seems to have totally kicked the bucket. My latest
tiddlyweb stuff is still on there so I'm going by memory...
On Aug 7, 8:40 am, Michael Mahemoff <mahem...@gmail.com> wrote:
> In the first instance, though, I will add
> some checks so it does not cause any error when the root tiddler is
> renamed.
to rename the root tiddler itself--ignoring for the time being what
happens to the comments on the root.
Are you able to reproduce this? From my sleuthing, the problem occurs
when the TiddlyWebAdaptor tries to "PUT" the renamed root tiddler. In
particular, trying to convert the root tiddler to JSON form is what
fails. The root tiddler has an attribute called firstChild which is
itself a tiddler object and that causes $.tojson() to blow up for some
reason. The firstChild attribute comes from the CommentTiddler's
treeifyComments() function. The offending tojson() call is in the
TiddlyWebAdaptor in adaptor.putTiddlerChronicle().
So the culprit looks to be that tiddlers with comments on them have
attributes that are themselves tiddler objects and that causes
conversion to JSON to fail.
I've tested removing the firstChild attribute from the root tiddler
before it's saved, and this does fix the problem, but there
are some additional complications in the way.
> It might be argued that's the desirable behaviour; that comments
> shouldn't automatically be moved upon rename. That said, I can see it
> would certainly be a useful feature, as an option.
confusing to have comments with titles that refer to a nonexistent
tiddler. Will have to experiment a bit and see though.
> I could possibly build a macro that renames a tiddler and updates
> comments, all in one.
working. I keep running into one problem after another though, so I
keep thinking I've almost got it working and then something else comes
up. At the moment it's stuck on my dead laptop so it'll take a little
time for me to get back to it.
By the way, is there any where you're doing current development of the
CommentsPlugin? If possible I'd like to stay updated with changes you
make, so I know problems I run into aren't unique to my version of the
plugin. I think there are now a couple versions around (tiddlyguv,
peermore, my version on tiddlycube).