creation Item object

[Michael Mayorov, 2011-07-08 19:04:08]

http://codereview.appspot.com/4675066/diff/2001/MoinMoin/storage/backends/ind...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/2001/MoinMoin/storage/backends/ind...
MoinMoin/storage/backends/indexing.py:399: print converter(doc_DOM) # XXX: It
isn't content!
Output:
2011-07-09 01:00:39,371 INFO MoinMoin.util.clock:40 timer conv_in_dom(0): 1.18ms
contenttype
name
language
tags
itemlinks
hostname
itemtransclusions
address
action
mtime
uuid

[ThomasWaldmann, 2011-07-08 19:29:08]

http://codereview.appspot.com/4675066/diff/2001/MoinMoin/storage/backends/ind...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/2001/MoinMoin/storage/backends/ind...
MoinMoin/storage/backends/indexing.py:396: item.create(name=name,
contenttype=contenttype, rev_no=revno)
please just check how this is used in other parts of the code.

http://codereview.appspot.com/4675066/diff/2001/MoinMoin/storage/backends/ind...
MoinMoin/storage/backends/indexing.py:399: print converter(doc_DOM) # XXX: It
isn't content!
On 2011/07/08 19:04:08, Michael Mayorov wrote:
> Output:
> 2011-07-09 01:00:39,371 INFO MoinMoin.util.clock:40 timer conv_in_dom(0):
1.18ms
> contenttype
> name
> language
> tags
> itemlinks
> hostname
> itemtransclusions
> address
> action
> mtime
> uuid
> 

maybe because you use it wrong?

[Michael Mayorov, 2011-07-08 19:51:11]

http://codereview.appspot.com/4675066/diff/2001/MoinMoin/storage/backends/ind...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/2001/MoinMoin/storage/backends/ind...
MoinMoin/storage/backends/indexing.py:396: item.create(name=name,
contenttype=contenttype, rev_no=revno)
On 2011/07/08 19:29:08, ThomasWaldmann wrote:
> please just check how this is used in other parts of the code.

For
        item = Item.create(name=name, rev_no=revno)
        doc_DOM = item.internal_representation()
        converter = MoinConv()
        print converter(doc_DOM) # XXX: It isn't content!

traceback:

http://paste.pocoo.org/show/433616/

[ThomasWaldmann, 2011-07-09 09:50:36]

http://codereview.appspot.com/4675066/diff/4/MoinMoin/storage/backends/indexi...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/4/MoinMoin/storage/backends/indexi...
MoinMoin/storage/backends/indexing.py:396: convert = MoinConv()
converter

http://codereview.appspot.com/4675066/diff/4/MoinMoin/storage/backends/indexi...
MoinMoin/storage/backends/indexing.py:397: data_converted = convert(dom)
what is the returned data type of this?

[Michael Mayorov, 2011-07-09 09:58:04]

http://codereview.appspot.com/4675066/diff/4/MoinMoin/storage/backends/indexi...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/4/MoinMoin/storage/backends/indexi...
MoinMoin/storage/backends/indexing.py:397: data_converted = convert(dom)
On 2011/07/09 09:50:36, ThomasWaldmann wrote:
> what is the returned data type of this?

Converter application/x.moin.document -> text/x.moin.wiki

[ThomasWaldmann, 2011-07-09 10:04:50]

http://codereview.appspot.com/4675066/diff/4/MoinMoin/storage/backends/indexi...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/4/MoinMoin/storage/backends/indexi...
MoinMoin/storage/backends/indexing.py:397: data_converted = convert(dom)
On 2011/07/09 09:58:05, Michael Mayorov wrote:
> On 2011/07/09 09:50:36, ThomasWaldmann wrote:
> > what is the returned data type of this?
> 
> Converter application/x.moin.document -> text/x.moin.wiki

i asked for the (python) data type.

http://codereview.appspot.com/4675066/diff/12001/MoinMoin/storage/backends/in...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/12001/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:397: data_converted =
convert(dom).encode('utf-8')
why encode? doesn't whoosh accept unicode objects?

[ThomasWaldmann, 2011-07-09 14:11:07]

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/converter/moinwiki_...
File MoinMoin/converter/moinwiki_out.py (right):

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/converter/moinwiki_...
MoinMoin/converter/moinwiki_out.py:179: href = u''
try href = unicode(href)

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/converter/moinwiki_...
MoinMoin/converter/moinwiki_out.py:187: args = u','.join(s for s in [args,
params] if s)
code above should be rewritten, but that is not your task

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/converter/moinwiki_...
MoinMoin/converter/moinwiki_out.py:299: href = u''
see above

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/converter/moinwiki_...
MoinMoin/converter/moinwiki_out.py:304: href = href[0].split(u'wiki.local:')[-1]
see above

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/storage/backends/in...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:395: item = Item.create(name=name,
rev_no=revno, contenttype=contenttype)
is it needed / good to give contenttype?

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:398: data_converted = converter(dom)
question from last review still unanswered

[Michael Mayorov, 2011-07-09 14:23:04]

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/storage/backends/in...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:395: item = Item.create(name=name,
rev_no=revno, contenttype=contenttype)
On 2011/07/09 14:11:08, ThomasWaldmann wrote:
> is it needed / good to give contenttype?

it's not needed but why I can't add it there?

http://codereview.appspot.com/4675066/diff/12002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:398: data_converted = converter(dom)
On 2011/07/09 14:11:08, ThomasWaldmann wrote:
> question from last review still unanswered

Whoosh accept unicode objects

[Reimar Bauer, 2011-07-09 22:27:55]

some comments

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/items/__init__.py
File MoinMoin/items/__init__.py (right):

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/items/__init__.py#n...
MoinMoin/items/__init__.py:230: except:
at least exact exception

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/items/__init__.py#n...
MoinMoin/items/__init__.py:269: except:
see aboce

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/script/maint/index.py
File MoinMoin/script/maint/index.py (right):

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/script/maint/index....
MoinMoin/script/maint/index.py:145: Note: before move new indexes this script
erasing previous, so watch
erases?

so be carefull? meaning of watch is unclear to me

[Michael Mayorov, 2011-07-09 22:32:13]

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/script/maint/index.py
File MoinMoin/script/maint/index.py (right):

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/script/maint/index....
MoinMoin/script/maint/index.py:145: Note: before move new indexes this script
erasing previous, so watch
On 2011/07/09 22:27:55, Reimar Bauer wrote:
> erases?
> 
> so be carefull? meaning of watch is unclear to me

e.g.
If you do:
moin index --for all-revs --action build
moin index --for both --action move

Then you erases all revisions indexes

[Michael Mayorov, 2011-07-09 22:33:17]

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/script/maint/index.py
File MoinMoin/script/maint/index.py (right):

http://codereview.appspot.com/4675066/diff/21001/MoinMoin/script/maint/index....
MoinMoin/script/maint/index.py:145: Note: before move new indexes this script
erasing previous, so watch
On 2011/07/09 22:32:13, Michael Mayorov wrote:
> On 2011/07/09 22:27:55, Reimar Bauer wrote:
> > erases?
> > 
> > so be carefull? meaning of watch is unclear to me
> 
> e.g.
> If you do:
> moin index --for all-revs --action build
> moin index --for both --action move
> 
> Then you erases all revisions indexes

s/all/latest/

[ThomasWaldmann, 2011-07-09 22:43:56]

http://codereview.appspot.com/4675066/diff/5/MoinMoin/items/__init__.py
File MoinMoin/items/__init__.py (right):

http://codereview.appspot.com/4675066/diff/5/MoinMoin/items/__init__.py#newco...
MoinMoin/items/__init__.py:229: flaskg.clock.start('conv_in_dom')
flaskg only works at request-time

from where do you call this at non-request-time?

http://codereview.appspot.com/4675066/diff/5/MoinMoin/items/__init__.py#newco...
MoinMoin/items/__init__.py:268: flaskg.clock.stop('conv_in_dom')
see above

[ThomasWaldmann, 2011-07-10 11:22:57]

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/script/maint/index.py
File MoinMoin/script/maint/index.py (right):

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/script/maint/index....
MoinMoin/script/maint/index.py:62: clean_index(indexnames_schemas)
we already talked about that a farm index has to be built from multiple wiki's
contents.

how does this work if you kill the index here?

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:36: converter = Converter()
no, don't do this globally.

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:549: def get_item_content(self, name,
rev_no):
convert_data()?

item is misleading, because you are dealing with revisions.

get...content is also misleading, because you are not getting the stored
content, but a converted version of it.

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:552: return converter(dom)
what is the returned python data type?

[Michael Mayorov, 2011-07-10 16:08:56]

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/script/maint/index.py
File MoinMoin/script/maint/index.py (right):

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/script/maint/index....
MoinMoin/script/maint/index.py:62: clean_index(indexnames_schemas)
On 2011/07/10 11:22:57, ThomasWaldmann wrote:
> we already talked about that a farm index has to be built from multiple wiki's
> contents.
> 
> how does this work if you kill the index here?

Sorry, I forgot about farm, will fix in next changeset

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:549: def get_item_content(self, name,
rev_no):
On 2011/07/10 11:22:57, ThomasWaldmann wrote:
> convert_data()?

ok

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:552: return converter(dom)
On 2011/07/10 11:22:57, ThomasWaldmann wrote:
> what is the returned python data type?

str

[Michael Mayorov, 2011-07-11 04:46:55]

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:552: return converter(dom)
On 2011/07/10 16:08:56, Michael Mayorov wrote:
> On 2011/07/10 11:22:57, ThomasWaldmann wrote:
> > what is the returned python data type?
> 
> str
oops,
s/str/unicode/

[ThomasWaldmann, 2011-07-11 09:25:22]

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/24002/MoinMoin/storage/backends/in...
MoinMoin/storage/backends/indexing.py:552: return converter(dom)
On 2011/07/11 04:46:55, Michael Mayorov wrote:
> On 2011/07/10 16:08:56, Michael Mayorov wrote:
> > On 2011/07/10 11:22:57, ThomasWaldmann wrote:
> > > what is the returned python data type?
> > 
> > str
> oops,
> s/str/unicode/

Was this a guess or did you check?

http://codereview.appspot.com/4675066/diff/6/MoinMoin/items/__init__.py
File MoinMoin/items/__init__.py (right):

http://codereview.appspot.com/4675066/diff/6/MoinMoin/items/__init__.py#newco...
MoinMoin/items/__init__.py:231: flaskg.clock.start('conv_in_dom')
no, that's nonsense

i pointed you at the flask test_request_context() docs recently and they
describe what you need to do.

http://flask.pocoo.org/docs/testing/#other-testing-tricks

read them again, maybe you read an older version.

http://codereview.appspot.com/4675066/diff/6/MoinMoin/items/__init__.py#newco...
MoinMoin/items/__init__.py:269: flaskg.clock.stop('conv_in_dom')
see above

http://codereview.appspot.com/4675066/diff/6/MoinMoin/storage/backends/indexi...
File MoinMoin/storage/backends/indexing.py (right):

http://codereview.appspot.com/4675066/diff/6/MoinMoin/storage/backends/indexi...
MoinMoin/storage/backends/indexing.py:548: def convert_data(self, name, rev_no):
you have that function duplicated in 2 modules?