Roundup Tracker - Issues

Issue 2550647

classification
Xapian backend: Xapian 1.2 compatibility
Type: Severity: normal
Components: Versions: 1.4, 1.3
process
Status: closed fixed
:
: : ber, bruce, olly
Priority: : patch

Created on 2010-04-30 13:47 by olly, last changed 2010-10-25 09:16 by bruce.

Files
File name Uploaded Description Edit Remove
roundup-1.4.13-xapian-1.2.patch olly, 2010-04-30 13:47
roundup-trunk-r4483-xapian-1.2-take2.patch olly, 2010-06-28 12:33 Patch take 2
Messages
msg4045 Author: [hidden] (olly) Date: 2010-04-30 13:47
I'm aiming to get Xapian 1.2 into the next Debian stable release, so I
am in the process of checking all the Debian packages which depend on
Xapian, one of which is roundup.

Xapian 1.2.0 drops support for some deprecated features, and roundup
uses one: MSetItem's get_docid() method should now be a docid property.
 This change is compatible with 1.0.x as well, but may not be with
0.9.x.  But Xapian 0.9.x is completely obsolete now so I just update the
install docs to give 1.0.0 as the minimum requirement.

Also, transactions have been supported for ages (since 0.9.7) so I
enabled those.

And MSet now implements __len__ so I made that change.

I'll attach a patch with these changes, but I'm afraid it is untested as
I don't actually use roundup, and I could see any unit tests which
covered this.  I hope it's useful as a pointer though.  If you need help
or advice on Xapian, feel free to ask.
msg4069 Author: [hidden] (ber) Date: 2010-06-28 10:00
Olly, Thanks for the patch!

I've tried it, against trunk Revision 4483 on Debian Lenny.
Patch applied cleanly.

ii  libxapian15    1.0.7-4        Search engine library
ii  python-xapian  1.0.7-3.1 

Unit tests fail, all 7 Xapian tests.
  python run_tests.py test_indexer
FAILED (failures=7)

Trying this at home is very easy, just get the latest version of 
roundup, and run the test.
msg4070 Author: [hidden] (ber) Date: 2010-06-28 10:06
Here is one of the failure, they all look similiar.
======================================================================
FAIL: test_basics (test.test_indexer.XapianIndexerTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.5/unittest.py", line 260, in run
    testMethod()
  
File "/home/bernhard/hacking/roundup/svn/trunk/./test/test_indexer.py", 
line 65, in test_basics
    ('test', '2', 'foo')])
  
File "/home/bernhard/hacking/roundup/svn/trunk/./test/test_indexer.py", 
line 59, in assertSeqEqual
    self.fail('contents of %r != %r'%(s1, s2))
  File "/usr/lib/python2.5/unittest.py", line 301, in fail
    raise self.failureException, msg
AssertionError: contents of [('test', '2', 'foo')] != 
[('test', '1', 'foo'), ('test', '2', 'foo')]
msg4071 Author: [hidden] (olly) Date: 2010-06-28 12:33
These failures are mostly because of enabling the commented out
transaction code - the roundup testsuite doesn't expect to have to call
save_index() to commit changes, but that is required when transactions
are used.  I've no idea how this is intended to work, so I just disabled
transactions again for now, and updated the comments instead.

There was however a mistake in the docid attribute change I made.  I've
attached an updated patch which makes roundup trunk r4483 pass with both
Xapian 1.0.12 and 1.2.2.
msg4072 Author: [hidden] (ber) Date: 2010-06-28 12:45
Olly, thanks for the new patch.
About the transaction, I believe the save() would have to be done
in the indexer_xapian.py somewhere not in the testsuite.
The test_indexer() uses the api that all indexers should use.

I don't know enough about xapian to advise here.
msg4073 Author: [hidden] (olly) Date: 2010-06-28 12:52
I would leave the transaction issue for now then - roundup will work
just as well with the second patch applied as it does currently, but
with the added bonus of working with Xapian 1.2.x, which beats breaking it.

It's quite possible whoever wrote this code thought they wanted to use
transactions, but found they weren't implemented at the time, and left
that commented out code there despite it not being needed at all.

The comments about why the transaction code isn't used are at least more
correct now, should anyone want to investigate in the future.
msg4074 Author: [hidden] (ber) Date: 2010-06-28 12:53
I've commited the change roundup-trunk-r4483-xapian-1.2-take2.patch    
to Revision 4484 with the minor change:

-        # XXX: Xapian databases don't actually implement transactions 
yet
+
+        # XXX: Xapian now supports transactions,
+        #  but there is a call to save_index() missing.

Olly, to close the issue, could you test if this works on Xapian 1.2
and could you provide a patch to make the transactions work or 
alternatively open a new issue for it?

Thanks in advance,
Bernhard
msg4077 Author: [hidden] (olly) Date: 2010-06-29 03:55
I've deleted my roundup tree already, but I tested the second patch with
Xapian 1.2 and you've only changed comments and blank lines, so testing
current trunk doesn't seem to add anything useful.

I'll leave the transaction issue to someone who knows the roundup code.
msg4078 Author: [hidden] (ber) Date: 2010-06-29 07:48
Olly, okay, so I am resolving the issue now.
Thanks again for contributing to Roundup!
Bernhard
History
Date User Action Args
2010-10-25 09:16:54brucesetnosy: + bruce
2010-06-29 07:48:19bersetstatus: open -> closed
2010-06-29 07:48:04bersetstatus: new -> open
resolution: fixed
messages: + msg4078
versions: + 1.4
2010-06-29 03:55:46ollysetmessages: + msg4077
2010-06-28 12:53:48bersetmessages: + msg4074
2010-06-28 12:52:32ollysetmessages: + msg4073
2010-06-28 12:45:51bersetmessages: + msg4072
2010-06-28 12:33:33ollysetfiles: + roundup-trunk-r4483-xapian-1.2-take2.patch
messages: + msg4071
2010-06-28 10:06:46bersetmessages: + msg4070
2010-06-28 10:00:43bersetnosy: + ber
messages: + msg4069
2010-04-30 13:47:34ollycreate