Issue 2550647
Created on 2010-04-30 13:47 by olly, last changed 2010-10-25 09:16 by bruce.
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
|
|
Date |
User |
Action |
Args |
2010-10-25 09:16:54 | bruce | set | nosy:
+ bruce |
2010-06-29 07:48:19 | ber | set | status: open -> closed |
2010-06-29 07:48:04 | ber | set | status: new -> open resolution: fixed messages:
+ msg4078 versions:
+ 1.4 |
2010-06-29 03:55:46 | olly | set | messages:
+ msg4077 |
2010-06-28 12:53:48 | ber | set | messages:
+ msg4074 |
2010-06-28 12:52:32 | olly | set | messages:
+ msg4073 |
2010-06-28 12:45:51 | ber | set | messages:
+ msg4072 |
2010-06-28 12:33:33 | olly | set | files:
+ roundup-trunk-r4483-xapian-1.2-take2.patch messages:
+ msg4071 |
2010-06-28 10:06:46 | ber | set | messages:
+ msg4070 |
2010-06-28 10:00:43 | ber | set | nosy:
+ ber messages:
+ msg4069 |
2010-04-30 13:47:34 | olly | create | |
|