You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bloodhound.apache.org by Saint Germain <sa...@gmail.com> on 2014/01/13 19:17:40 UTC
Whoosh requirements
Hello,
Apparently for bloodhound_search, a specific version of Whoosh is
required (2.4.1).
I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
apparently fix some problem in Whoosh (pull request #41).
Apparently this pull request is already included in Whoosh trunk
(since 2.5.1), so I was wondering if the fix is always necessary and if
Whoosh version > 2.5.1 may be acceptable.
Is there a thread on the topic where I can understand the problem and
attempt to fix it ?
Thanks !
Re: Whoosh requirements
Posted by Anže Starič <an...@gmail.com>.
> Can someone review the patch and check that it is correct ?
The patch looks ok. As far as I am concerned, you can commit it.
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
On Wed, 15 Jan 2014 10:53:01 +0100, Andrej Golcov <an...@digiverse.si>
wrote :
> >> > I would suggest to make Bloodhound using Whoosh 2.5 (and removing
> >> > aforementioned workarounds) with search index rebuild when
> >> > upgrading from setup that uses indexes created with Whoosh 2.4.
> >> >
> >>
> >> Hello,
> >>
> >> I have created the ticket #741 to manage this point:
> >> https://issues.apache.org/bloodhound/ticket/741
> >>
> >> I added a tentative patch to remove the workaround and
> >> remove/modify the tests accordingly.
> >>
> >> I have no experience on Bloodhound upgrade, so I haven't touched
> >> anything on this part.
> >> I also intend to manually test the search features of Bloodhound
> >> in the next days to check if anything is broken by this Whoosh
> >> upgrade (but having read the 2.5 changelog, it seems that there
> >> are no big changes).
> >>
> >> Best regards,
> >>
> >
> > Should we add Whoosh (and it's version) as a requirement in
> > `requirements.txt` and `requirements-dev.txt`, since
> > BloodhoundSearch is specified in that file? For other packages such
> > as Trac we've included the dependencies in the pip requirements
> > file.
>
> requirements.txt references bloodhound_search that declares dependency
> on whoosh==2.4.1.
> See bloodhound_search/setup.py:
> In order to upgrade whoosh version, this define should be changed:
>
> install_requires = [
> 'setuptools>=0.6b1',
> 'Trac>=0.11',
> 'whoosh==2.4.1'
>
Hello !
I have uploaded a patch to ticket #741 against the current Bloodhound
svn trunk revision r1562687.
I have not modified requirements.txt and requirements-dev.txt as Andrej
explained that it should be modified only in bloodhound_search/setup.py.
Can someone review the patch and check that it is correct ?
In particular I have a question on how to handle exceptions coming from
Whoosh (currently it also "breaks" Bloodhound).
It would be good if it could be included in the next Bloodhound release.
Thanks in advance,
Re: Whoosh requirements
Posted by Andrej Golcov <an...@digiverse.si>.
requirements.txt references bloodhound_search that declares dependency
on whoosh==2.4.1.
See bloodhound_search/setup.py:
In order to upgrade whoosh version, this define should be changed:
install_requires = [
'setuptools>=0.6b1',
'Trac>=0.11',
'whoosh==2.4.1'
Cheers, Andrej
On 15 January 2014 02:50, Ryan Ollos <ry...@wandisco.com> wrote:
> On Tue, Jan 14, 2014 at 5:42 PM, Saint Germain <sa...@gmail.com> wrote:
>
>> On Tue, 14 Jan 2014 16:14:06 +0100, Andrej Golcov <an...@digiverse.si>
>> wrote :
>>
>> > I would suggest to make Bloodhound using Whoosh 2.5 (and removing
>> > aforementioned workarounds) with search index rebuild when upgrading
>> > from setup that uses indexes created with Whoosh 2.4.
>> >
>>
>> Hello,
>>
>> I have created the ticket #741 to manage this point:
>> https://issues.apache.org/bloodhound/ticket/741
>>
>> I added a tentative patch to remove the workaround and remove/modify
>> the tests accordingly.
>>
>> I have no experience on Bloodhound upgrade, so I haven't touched
>> anything on this part.
>> I also intend to manually test the search features of Bloodhound in the
>> next days to check if anything is broken by this Whoosh upgrade (but
>> having read the 2.5 changelog, it seems that there are no big changes).
>>
>> Best regards,
>>
>
> Should we add Whoosh (and it's version) as a requirement in
> `requirements.txt` and `requirements-dev.txt`, since BloodhoundSearch is
> specified in that file? For other packages such as Trac we've included the
> dependencies in the pip requirements file.
Re: Whoosh requirements
Posted by Olemis Lang <ol...@gmail.com>.
On 1/14/14, Ryan Ollos <ry...@wandisco.com> wrote:
> On Tue, Jan 14, 2014 at 5:42 PM, Saint Germain <sa...@gmail.com> wrote:
>
>> On Tue, 14 Jan 2014 16:14:06 +0100, Andrej Golcov <an...@digiverse.si>
>> wrote :
>>
>> > I would suggest to make Bloodhound using Whoosh 2.5 (and removing
>> > aforementioned workarounds) with search index rebuild when upgrading
>> > from setup that uses indexes created with Whoosh 2.4.
>> >
>>
>> Hello,
>>
>> I have created the ticket #741 to manage this point:
>> https://issues.apache.org/bloodhound/ticket/741
>>
>> I added a tentative patch to remove the workaround and remove/modify
>> the tests accordingly.
>>
>> I have no experience on Bloodhound upgrade, so I haven't touched
>> anything on this part.
>> I also intend to manually test the search features of Bloodhound in the
>> next days to check if anything is broken by this Whoosh upgrade (but
>> having read the 2.5 changelog, it seems that there are no big changes).
>>
>> Best regards,
>>
>
> Should we add Whoosh (and it's version) as a requirement in
> `requirements.txt` and `requirements-dev.txt`, since BloodhoundSearch is
> specified in that file? For other packages such as Trac we've included the
> dependencies in the pip requirements file.
>
I guess so , cmiiw . I really do not understand why this was not done
before so maybe there is a reason for not including it ?
--
Regards,
Olemis - @olemislc
Re: Whoosh requirements
Posted by Ryan Ollos <ry...@wandisco.com>.
On Tue, Jan 14, 2014 at 5:42 PM, Saint Germain <sa...@gmail.com> wrote:
> On Tue, 14 Jan 2014 16:14:06 +0100, Andrej Golcov <an...@digiverse.si>
> wrote :
>
> > I would suggest to make Bloodhound using Whoosh 2.5 (and removing
> > aforementioned workarounds) with search index rebuild when upgrading
> > from setup that uses indexes created with Whoosh 2.4.
> >
>
> Hello,
>
> I have created the ticket #741 to manage this point:
> https://issues.apache.org/bloodhound/ticket/741
>
> I added a tentative patch to remove the workaround and remove/modify
> the tests accordingly.
>
> I have no experience on Bloodhound upgrade, so I haven't touched
> anything on this part.
> I also intend to manually test the search features of Bloodhound in the
> next days to check if anything is broken by this Whoosh upgrade (but
> having read the 2.5 changelog, it seems that there are no big changes).
>
> Best regards,
>
Should we add Whoosh (and it's version) as a requirement in
`requirements.txt` and `requirements-dev.txt`, since BloodhoundSearch is
specified in that file? For other packages such as Trac we've included the
dependencies in the pip requirements file.
Re: Whoosh requirements
Posted by Olemis Lang <ol...@gmail.com>.
On 1/19/14, Saint Germain <sa...@gmail.com> wrote:
[...]
> Hello,
>
:)
[...]
>
> Currently on trunk I got a lot of similar mistakes when running the
> test on bloodhound_search like this one:
>
> ======================================================================
> ERROR: test_admin_granted_in_product_should_not_have_access
> (bhsearch.tests.security.MultiProductSecurityTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
[...]
> OperationalError: duplicate column name: product
>
> Does that ring a bell to someone ?
>
It looks ok to me , see http://pastebin.com/kxvua4TU
--
Regards,
Olemis - @olemislc
Re: Whoosh requirements
Posted by Ryan Ollos <ry...@wandisco.com>.
On Wed, Jan 22, 2014 at 5:38 PM, Saint Germain <sa...@gmail.com> wrote:
> On Wed, 22 Jan 2014 17:29:41 -0800, Ryan Ollos
> <ry...@wandisco.com> wrote :
>
> > On Wed, Jan 22, 2014 at 5:22 PM, Saint Germain <sa...@gmail.com>
> > wrote:
> >
> > >
> > >
> > > On Wed, 22 Jan 2014 16:45:19 -0800, Ryan Ollos
> > > <ry...@wandisco.com> wrote :
> > >
> > > > On Wed, Jan 22, 2014 at 4:27 PM, Saint Germain
> > > > <sa...@gmail.com> wrote:
> > > >
> > > > > On Mon, 20 Jan 2014 00:10:55 -0500, Olemis Lang
> > > > > <ol...@gmail.com> wrote :
> > > > >
> > > > > > >
> > > > > > > Currently on trunk I got a lot of similar mistakes when
> > > > > > > running the test on bloodhound_search like this one:
> > > > > > >
> > > > > > >
> > > ======================================================================
> > > > > > > ERROR: test_admin_granted_in_product_should_not_have_access
> > > > > > > (bhsearch.tests.security.MultiProductSecurityTestCase)
> > > > > > >
> > > ----------------------------------------------------------------------
> > > > > > > Traceback (most recent call last):
> > > > > > [...]
> > > > > > > OperationalError: duplicate column name: product
> > > > > > >
> > > > > > > Does that ring a bell to someone ?
> > > > > > >
> > > > > >
> > > > > > It looks ok to me , see http://pastebin.com/kxvua4TU
> > > > > >
> > > > >
> > > > > Hello !
> > > > >
> > > > > I found the problem.
> > > > >
> > > > > In tests/env.py we have:
> > > > > from sqlite3 import OperationalError
> > > > >
> > > > > And in trac/trac/db/sqlite_backend.py we have:
> > > > > try:
> > > > > import pysqlite2.dbapi2 as sqlite
> > > > > have_pysqlite = 2
> > > > > except ImportError:
> > > > > try:
> > > > > import sqlite3 as sqlite
> > > > > have_pysqlite = 2
> > > > > except ImportError:
> > > > > have_pysqlite = 0
> > > > >
> > > > > As I have both sqlite3 and python-pysqlite2 installed, I got a
> > > > > mismatch when an exception is raised:
> > > > > sqlite3.OperationalError != pysqlite2.dbapi2.OperationalError
> > > > >
> > > > > And the tests cannot run.
> > > > >
> > > > > If I deinstall python-pysqlite2, then the tests can run.
> > > > >
> > > > > Do you think it is necessary to fix this ?
> > > > > In test files we can reproduce the same logic as in the
> > > > > sqlite_backend to correctly import sqlite.
> > > > >
> > > > > Best regards,
> > > > >
> > > > >
> > > > Trac provides an API for working with database exceptions. I
> > > > believe the following patch would be the proper way to handle it.
> > > > Would you kindly test it in your environment?
> > > >
> > > > diff --git a/bloodhound_multiproduct/tests/env.py
> > > > b/bloodhound_multiproduct/tests/env.py
> > > > index 30910e4..98cd1bc 100644
> > > > --- a/bloodhound_multiproduct/tests/env.py
> > > > +++ b/bloodhound_multiproduct/tests/env.py
> > > > @@ -23,7 +23,6 @@ from inspect import stack
> > > > import os.path
> > > > import shutil
> > > > import tempfile
> > > > -from sqlite3 import OperationalError
> > > > from tests import unittest
> > > > from types import MethodType
> > > >
> > > > @@ -230,7 +229,7 @@ class MultiproductTestCase(unittest.TestCase):
> > > > mpsystem = MultiProductSystem(env)
> > > > try:
> > > > mpsystem.upgrade_environment(env.db_transaction)
> > > > - except OperationalError:
> > > > + except env.db_exc.OperationalError:
> > > > # Database is upgraded, but database version was
> > > > deleted. # Complete the upgrade by inserting default product.
> > > > mpsystem._insert_default_product(env.db_transaction)
> > >
> > > Hello !
> > >
> > > Thanks for the patch. I had to propagate the change in order to run
> > > all the tests.
> > >
> >
> > Looks good, thanks for testing. Do you have commit access now? If so,
> > I think you should feel free to go ahead and commit the change.
>
> Unfortunately not, I haven't sent the paperwork to get the commit
> access.
> I _will_ do it tomorrow.
>
> Thanks
>
I committed the change in 1560582. Thanks for finishing up the patch!
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
On Wed, 22 Jan 2014 17:29:41 -0800, Ryan Ollos
<ry...@wandisco.com> wrote :
> On Wed, Jan 22, 2014 at 5:22 PM, Saint Germain <sa...@gmail.com>
> wrote:
>
> >
> >
> > On Wed, 22 Jan 2014 16:45:19 -0800, Ryan Ollos
> > <ry...@wandisco.com> wrote :
> >
> > > On Wed, Jan 22, 2014 at 4:27 PM, Saint Germain
> > > <sa...@gmail.com> wrote:
> > >
> > > > On Mon, 20 Jan 2014 00:10:55 -0500, Olemis Lang
> > > > <ol...@gmail.com> wrote :
> > > >
> > > > > >
> > > > > > Currently on trunk I got a lot of similar mistakes when
> > > > > > running the test on bloodhound_search like this one:
> > > > > >
> > > > > >
> > ======================================================================
> > > > > > ERROR: test_admin_granted_in_product_should_not_have_access
> > > > > > (bhsearch.tests.security.MultiProductSecurityTestCase)
> > > > > >
> > ----------------------------------------------------------------------
> > > > > > Traceback (most recent call last):
> > > > > [...]
> > > > > > OperationalError: duplicate column name: product
> > > > > >
> > > > > > Does that ring a bell to someone ?
> > > > > >
> > > > >
> > > > > It looks ok to me , see http://pastebin.com/kxvua4TU
> > > > >
> > > >
> > > > Hello !
> > > >
> > > > I found the problem.
> > > >
> > > > In tests/env.py we have:
> > > > from sqlite3 import OperationalError
> > > >
> > > > And in trac/trac/db/sqlite_backend.py we have:
> > > > try:
> > > > import pysqlite2.dbapi2 as sqlite
> > > > have_pysqlite = 2
> > > > except ImportError:
> > > > try:
> > > > import sqlite3 as sqlite
> > > > have_pysqlite = 2
> > > > except ImportError:
> > > > have_pysqlite = 0
> > > >
> > > > As I have both sqlite3 and python-pysqlite2 installed, I got a
> > > > mismatch when an exception is raised:
> > > > sqlite3.OperationalError != pysqlite2.dbapi2.OperationalError
> > > >
> > > > And the tests cannot run.
> > > >
> > > > If I deinstall python-pysqlite2, then the tests can run.
> > > >
> > > > Do you think it is necessary to fix this ?
> > > > In test files we can reproduce the same logic as in the
> > > > sqlite_backend to correctly import sqlite.
> > > >
> > > > Best regards,
> > > >
> > > >
> > > Trac provides an API for working with database exceptions. I
> > > believe the following patch would be the proper way to handle it.
> > > Would you kindly test it in your environment?
> > >
> > > diff --git a/bloodhound_multiproduct/tests/env.py
> > > b/bloodhound_multiproduct/tests/env.py
> > > index 30910e4..98cd1bc 100644
> > > --- a/bloodhound_multiproduct/tests/env.py
> > > +++ b/bloodhound_multiproduct/tests/env.py
> > > @@ -23,7 +23,6 @@ from inspect import stack
> > > import os.path
> > > import shutil
> > > import tempfile
> > > -from sqlite3 import OperationalError
> > > from tests import unittest
> > > from types import MethodType
> > >
> > > @@ -230,7 +229,7 @@ class MultiproductTestCase(unittest.TestCase):
> > > mpsystem = MultiProductSystem(env)
> > > try:
> > > mpsystem.upgrade_environment(env.db_transaction)
> > > - except OperationalError:
> > > + except env.db_exc.OperationalError:
> > > # Database is upgraded, but database version was
> > > deleted. # Complete the upgrade by inserting default product.
> > > mpsystem._insert_default_product(env.db_transaction)
> >
> > Hello !
> >
> > Thanks for the patch. I had to propagate the change in order to run
> > all the tests.
> >
>
> Looks good, thanks for testing. Do you have commit access now? If so,
> I think you should feel free to go ahead and commit the change.
Unfortunately not, I haven't sent the paperwork to get the commit
access.
I _will_ do it tomorrow.
Thanks
Re: Whoosh requirements
Posted by Ryan Ollos <ry...@wandisco.com>.
On Wed, Jan 22, 2014 at 5:22 PM, Saint Germain <sa...@gmail.com> wrote:
>
>
> On Wed, 22 Jan 2014 16:45:19 -0800, Ryan Ollos
> <ry...@wandisco.com> wrote :
>
> > On Wed, Jan 22, 2014 at 4:27 PM, Saint Germain <sa...@gmail.com>
> > wrote:
> >
> > > On Mon, 20 Jan 2014 00:10:55 -0500, Olemis Lang <ol...@gmail.com>
> > > wrote :
> > >
> > > > >
> > > > > Currently on trunk I got a lot of similar mistakes when running
> > > > > the test on bloodhound_search like this one:
> > > > >
> > > > >
> ======================================================================
> > > > > ERROR: test_admin_granted_in_product_should_not_have_access
> > > > > (bhsearch.tests.security.MultiProductSecurityTestCase)
> > > > >
> ----------------------------------------------------------------------
> > > > > Traceback (most recent call last):
> > > > [...]
> > > > > OperationalError: duplicate column name: product
> > > > >
> > > > > Does that ring a bell to someone ?
> > > > >
> > > >
> > > > It looks ok to me , see http://pastebin.com/kxvua4TU
> > > >
> > >
> > > Hello !
> > >
> > > I found the problem.
> > >
> > > In tests/env.py we have:
> > > from sqlite3 import OperationalError
> > >
> > > And in trac/trac/db/sqlite_backend.py we have:
> > > try:
> > > import pysqlite2.dbapi2 as sqlite
> > > have_pysqlite = 2
> > > except ImportError:
> > > try:
> > > import sqlite3 as sqlite
> > > have_pysqlite = 2
> > > except ImportError:
> > > have_pysqlite = 0
> > >
> > > As I have both sqlite3 and python-pysqlite2 installed, I got a
> > > mismatch when an exception is raised:
> > > sqlite3.OperationalError != pysqlite2.dbapi2.OperationalError
> > >
> > > And the tests cannot run.
> > >
> > > If I deinstall python-pysqlite2, then the tests can run.
> > >
> > > Do you think it is necessary to fix this ?
> > > In test files we can reproduce the same logic as in the
> > > sqlite_backend to correctly import sqlite.
> > >
> > > Best regards,
> > >
> > >
> > Trac provides an API for working with database exceptions. I believe
> > the following patch would be the proper way to handle it. Would you
> > kindly test it in your environment?
> >
> > diff --git a/bloodhound_multiproduct/tests/env.py
> > b/bloodhound_multiproduct/tests/env.py
> > index 30910e4..98cd1bc 100644
> > --- a/bloodhound_multiproduct/tests/env.py
> > +++ b/bloodhound_multiproduct/tests/env.py
> > @@ -23,7 +23,6 @@ from inspect import stack
> > import os.path
> > import shutil
> > import tempfile
> > -from sqlite3 import OperationalError
> > from tests import unittest
> > from types import MethodType
> >
> > @@ -230,7 +229,7 @@ class MultiproductTestCase(unittest.TestCase):
> > mpsystem = MultiProductSystem(env)
> > try:
> > mpsystem.upgrade_environment(env.db_transaction)
> > - except OperationalError:
> > + except env.db_exc.OperationalError:
> > # Database is upgraded, but database version was deleted.
> > # Complete the upgrade by inserting default product.
> > mpsystem._insert_default_product(env.db_transaction)
>
> Hello !
>
> Thanks for the patch. I had to propagate the change in order to run all
> the tests.
>
Looks good, thanks for testing. Do you have commit access now? If so, I
think you should feel free to go ahead and commit the change.
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
On Wed, 22 Jan 2014 16:45:19 -0800, Ryan Ollos
<ry...@wandisco.com> wrote :
> On Wed, Jan 22, 2014 at 4:27 PM, Saint Germain <sa...@gmail.com>
> wrote:
>
> > On Mon, 20 Jan 2014 00:10:55 -0500, Olemis Lang <ol...@gmail.com>
> > wrote :
> >
> > > >
> > > > Currently on trunk I got a lot of similar mistakes when running
> > > > the test on bloodhound_search like this one:
> > > >
> > > > ======================================================================
> > > > ERROR: test_admin_granted_in_product_should_not_have_access
> > > > (bhsearch.tests.security.MultiProductSecurityTestCase)
> > > > ----------------------------------------------------------------------
> > > > Traceback (most recent call last):
> > > [...]
> > > > OperationalError: duplicate column name: product
> > > >
> > > > Does that ring a bell to someone ?
> > > >
> > >
> > > It looks ok to me , see http://pastebin.com/kxvua4TU
> > >
> >
> > Hello !
> >
> > I found the problem.
> >
> > In tests/env.py we have:
> > from sqlite3 import OperationalError
> >
> > And in trac/trac/db/sqlite_backend.py we have:
> > try:
> > import pysqlite2.dbapi2 as sqlite
> > have_pysqlite = 2
> > except ImportError:
> > try:
> > import sqlite3 as sqlite
> > have_pysqlite = 2
> > except ImportError:
> > have_pysqlite = 0
> >
> > As I have both sqlite3 and python-pysqlite2 installed, I got a
> > mismatch when an exception is raised:
> > sqlite3.OperationalError != pysqlite2.dbapi2.OperationalError
> >
> > And the tests cannot run.
> >
> > If I deinstall python-pysqlite2, then the tests can run.
> >
> > Do you think it is necessary to fix this ?
> > In test files we can reproduce the same logic as in the
> > sqlite_backend to correctly import sqlite.
> >
> > Best regards,
> >
> >
> Trac provides an API for working with database exceptions. I believe
> the following patch would be the proper way to handle it. Would you
> kindly test it in your environment?
>
> diff --git a/bloodhound_multiproduct/tests/env.py
> b/bloodhound_multiproduct/tests/env.py
> index 30910e4..98cd1bc 100644
> --- a/bloodhound_multiproduct/tests/env.py
> +++ b/bloodhound_multiproduct/tests/env.py
> @@ -23,7 +23,6 @@ from inspect import stack
> import os.path
> import shutil
> import tempfile
> -from sqlite3 import OperationalError
> from tests import unittest
> from types import MethodType
>
> @@ -230,7 +229,7 @@ class MultiproductTestCase(unittest.TestCase):
> mpsystem = MultiProductSystem(env)
> try:
> mpsystem.upgrade_environment(env.db_transaction)
> - except OperationalError:
> + except env.db_exc.OperationalError:
> # Database is upgraded, but database version was deleted.
> # Complete the upgrade by inserting default product.
> mpsystem._insert_default_product(env.db_transaction)
Hello !
Thanks for the patch. I had to propagate the change in order to run all
the tests.
So I suppose that the final patch for this problem is the following:
diff -r 28dff22568f4 bloodhound_multiproduct/multiproduct/env.py
--- a/bloodhound_multiproduct/multiproduct/env.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_multiproduct/multiproduct/env.py Thu Jan 23 02:20:30 2014 +0100
@@ -21,7 +21,6 @@
import os.path
from urlparse import urlsplit
-from sqlite3 import OperationalError
from trac.config import BoolOption, ConfigSection, Option
from trac.core import Component, ComponentManager, ExtensionPoint, implements, \
diff -r 28dff22568f4 bloodhound_multiproduct/tests/env.py
--- a/bloodhound_multiproduct/tests/env.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_multiproduct/tests/env.py Thu Jan 23 02:20:30 2014 +0100
@@ -23,7 +23,6 @@
import os.path
import shutil
import tempfile
-from sqlite3 import OperationalError
from tests import unittest
from types import MethodType
@@ -230,7 +229,7 @@
mpsystem = MultiProductSystem(env)
try:
mpsystem.upgrade_environment(env.db_transaction)
- except OperationalError:
+ except env.db_exc.OperationalError:
# Database is upgraded, but database version was deleted.
# Complete the upgrade by inserting default product.
mpsystem._insert_default_product(env.db_transaction)
@@ -310,7 +309,7 @@
if self.env is not None:
try:
self.env.reset_db()
- except OperationalError:
+ except self.env.db_exc.OperationalError:
# "Database not found ...",
# "OperationalError: no such table: system" or the like
pass
@@ -575,7 +574,7 @@
if self.env is not None:
try:
self.env.reset_db()
- except OperationalError:
+ except self.env.db_exc.OperationalError:
# "Database not found ...",
# "OperationalError: no such table: system" or the like
pass
diff -r 28dff22568f4 bloodhound_multiproduct/tests/model.py
--- a/bloodhound_multiproduct/tests/model.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_multiproduct/tests/model.py Thu Jan 23 02:20:30 2014 +0100
@@ -19,7 +19,6 @@
"""Tests for multiproduct/model.py"""
import shutil
import tempfile
-from sqlite3 import OperationalError
from tests import unittest
from trac.core import TracError
@@ -46,7 +45,7 @@
self.mpsystem = MultiProductSystem(self.env)
try:
self.mpsystem.upgrade_environment(self.env.db_transaction)
- except OperationalError:
+ except self.env.db_exc.OperationalError:
# table remains but database version is deleted
pass
diff -r 28dff22568f4 bloodhound_multiproduct/tests/upgrade.py
--- a/bloodhound_multiproduct/tests/upgrade.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_multiproduct/tests/upgrade.py Thu Jan 23 02:20:30 2014 +0100
@@ -21,7 +21,6 @@
import shutil
import tempfile
import uuid
-from sqlite3 import OperationalError
from contextlib import contextmanager
from tests import unittest
@@ -434,13 +433,13 @@
@contextmanager
def assertFailsWithMissingTable(self):
- with self.assertRaises(OperationalError) as cm:
+ with self.assertRaises(self.env.db_exc.OperationalError) as cm:
yield
self.assertIn('no such table', str(cm.exception))
@contextmanager
def assertFailsWithMissingColumn(self):
- with self.assertRaises(OperationalError) as cm:
+ with self.assertRaises(self.env.db_exc.OperationalError) as cm:
yield
self.assertIn('no such column', str(cm.exception))
diff -r 28dff22568f4 bloodhound_relations/bhrelations/search.py
--- a/bloodhound_relations/bhrelations/search.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_relations/bhrelations/search.py Thu Jan 23 02:20:30 2014 +0100
@@ -18,8 +18,6 @@
# specific language governing permissions and limitations
# under the License.
-from sqlite3 import OperationalError
-
from trac.core import Component, implements
from bhsearch.api import IDocIndexPreprocessor
@@ -42,7 +40,7 @@
for relation in rls._select_relations(resource_id):
relations.extend(self._format_relations(relation))
doc['relations'] = ','.join(relations)
- except OperationalError:
+ except self.env.db_exc.OperationalError:
# If bhrelations and bhsearch are installed at the same time and
# bhsearch is upgraded before bhrelations, table
# bloodhound_relations will be missing, thus causing the
diff -r 28dff22568f4 bloodhound_relations/bhrelations/tests/api.py
--- a/bloodhound_relations/bhrelations/tests/api.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_relations/bhrelations/tests/api.py Thu Jan 23 02:20:30 2014 +0100
@@ -18,7 +18,6 @@
# specific language governing permissions and limitations
# under the License.
from datetime import datetime
-from _sqlite3 import IntegrityError
import unittest
from bhrelations.api import TicketRelationsSpecifics
from bhrelations.tests.mocks import TestRelationChangingListener
@@ -98,7 +97,10 @@
with self.env.db_transaction as db:
db(sql, ["1", "2", "dependson"])
self.assertRaises(
- IntegrityError, db, sql, ["1", "2", "dependson"])
+ self.env.db_exc.IntegrityError,
+ db,
+ sql,
+ ["1", "2", "dependson"])
def test_can_add_one_way_relations(self):
#arrange
diff -r 28dff22568f4 bloodhound_relations/bhrelations/tests/base.py
--- a/bloodhound_relations/bhrelations/tests/base.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_relations/bhrelations/tests/base.py Thu Jan 23 02:20:30 2014 +0100
@@ -15,7 +15,6 @@
# specific language governing permissions and limitations
# under the License.
-from _sqlite3 import OperationalError
from tests.env import MultiproductTestCase
from multiproduct.env import ProductEnvironment
from bhrelations.api import RelationsSystem, EnvironmentSetup, \
@@ -86,7 +85,7 @@
environment_setup = EnvironmentSetup(self.env)
try:
environment_setup.upgrade_environment(self.env.db_transaction)
- except OperationalError:
+ except self.env.db_exc.OperationalError:
# table remains but database version is deleted
pass
diff -r 28dff22568f4 bloodhound_search/bhsearch/tests/security.py
--- a/bloodhound_search/bhsearch/tests/security.py Thu Jan 23 00:29:55 2014 +0100
+++ b/bloodhound_search/bhsearch/tests/security.py Thu Jan 23 02:20:30 2014 +0100
@@ -24,7 +24,6 @@
"""
import contextlib
import os
-from sqlite3 import OperationalError
from bhsearch.security import SecurityFilter
try:
@@ -69,7 +68,7 @@
try:
MultiProductSystem(self.env)\
.upgrade_environment(self.env.db_transaction)
- except OperationalError:
+ except self.env.db_exc.OperationalError:
# table remains but content is deleted
self._add_products('@')
self.env.enable_multiproduct_schema()
Re: Whoosh requirements
Posted by Ryan Ollos <ry...@wandisco.com>.
On Wed, Jan 22, 2014 at 4:27 PM, Saint Germain <sa...@gmail.com> wrote:
> On Mon, 20 Jan 2014 00:10:55 -0500, Olemis Lang <ol...@gmail.com>
> wrote :
>
> > >
> > > Currently on trunk I got a lot of similar mistakes when running the
> > > test on bloodhound_search like this one:
> > >
> > > ======================================================================
> > > ERROR: test_admin_granted_in_product_should_not_have_access
> > > (bhsearch.tests.security.MultiProductSecurityTestCase)
> > > ----------------------------------------------------------------------
> > > Traceback (most recent call last):
> > [...]
> > > OperationalError: duplicate column name: product
> > >
> > > Does that ring a bell to someone ?
> > >
> >
> > It looks ok to me , see http://pastebin.com/kxvua4TU
> >
>
> Hello !
>
> I found the problem.
>
> In tests/env.py we have:
> from sqlite3 import OperationalError
>
> And in trac/trac/db/sqlite_backend.py we have:
> try:
> import pysqlite2.dbapi2 as sqlite
> have_pysqlite = 2
> except ImportError:
> try:
> import sqlite3 as sqlite
> have_pysqlite = 2
> except ImportError:
> have_pysqlite = 0
>
> As I have both sqlite3 and python-pysqlite2 installed, I got a mismatch
> when an exception is raised:
> sqlite3.OperationalError != pysqlite2.dbapi2.OperationalError
>
> And the tests cannot run.
>
> If I deinstall python-pysqlite2, then the tests can run.
>
> Do you think it is necessary to fix this ?
> In test files we can reproduce the same logic as in the sqlite_backend
> to correctly import sqlite.
>
> Best regards,
>
>
Trac provides an API for working with database exceptions. I believe the
following patch would be the proper way to handle it. Would you kindly test
it in your environment?
diff --git a/bloodhound_multiproduct/tests/env.py
b/bloodhound_multiproduct/tests/env.py
index 30910e4..98cd1bc 100644
--- a/bloodhound_multiproduct/tests/env.py
+++ b/bloodhound_multiproduct/tests/env.py
@@ -23,7 +23,6 @@ from inspect import stack
import os.path
import shutil
import tempfile
-from sqlite3 import OperationalError
from tests import unittest
from types import MethodType
@@ -230,7 +229,7 @@ class MultiproductTestCase(unittest.TestCase):
mpsystem = MultiProductSystem(env)
try:
mpsystem.upgrade_environment(env.db_transaction)
- except OperationalError:
+ except env.db_exc.OperationalError:
# Database is upgraded, but database version was deleted.
# Complete the upgrade by inserting default product.
mpsystem._insert_default_product(env.db_transaction)
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
On Mon, 20 Jan 2014 00:10:55 -0500, Olemis Lang <ol...@gmail.com>
wrote :
> >
> > Currently on trunk I got a lot of similar mistakes when running the
> > test on bloodhound_search like this one:
> >
> > ======================================================================
> > ERROR: test_admin_granted_in_product_should_not_have_access
> > (bhsearch.tests.security.MultiProductSecurityTestCase)
> > ----------------------------------------------------------------------
> > Traceback (most recent call last):
> [...]
> > OperationalError: duplicate column name: product
> >
> > Does that ring a bell to someone ?
> >
>
> It looks ok to me , see http://pastebin.com/kxvua4TU
>
Hello !
I found the problem.
In tests/env.py we have:
from sqlite3 import OperationalError
And in trac/trac/db/sqlite_backend.py we have:
try:
import pysqlite2.dbapi2 as sqlite
have_pysqlite = 2
except ImportError:
try:
import sqlite3 as sqlite
have_pysqlite = 2
except ImportError:
have_pysqlite = 0
As I have both sqlite3 and python-pysqlite2 installed, I got a mismatch
when an exception is raised:
sqlite3.OperationalError != pysqlite2.dbapi2.OperationalError
And the tests cannot run.
If I deinstall python-pysqlite2, then the tests can run.
Do you think it is necessary to fix this ?
In test files we can reproduce the same logic as in the sqlite_backend
to correctly import sqlite.
Best regards,
Re: Whoosh requirements
Posted by Olemis Lang <ol...@gmail.com>.
On 1/19/14, Saint Germain <sa...@gmail.com> wrote:
[...]
> Hello,
>
:)
[...]
>
> Currently on trunk I got a lot of similar mistakes when running the
> test on bloodhound_search like this one:
>
> ======================================================================
> ERROR: test_admin_granted_in_product_should_not_have_access
> (bhsearch.tests.security.MultiProductSecurityTestCase)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
[...]
> OperationalError: duplicate column name: product
>
> Does that ring a bell to someone ?
>
It looks ok to me , see http://pastebin.com/kxvua4TU
--
Regards,
Olemis - @olemislc
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
On Tue, 14 Jan 2014 23:40:01 -0500, Olemis Lang <ol...@gmail.com>
wrote :
> On 1/14/14, Saint Germain <sa...@gmail.com> wrote:
> > On Tue, 14 Jan 2014 16:14:06 +0100, Andrej Golcov
> > <an...@digiverse.si> wrote :
> >
> [...]
> > I also intend to manually test the search features of Bloodhound in
> > the next days to check if anything is broken by this Whoosh upgrade
>
> It'd be helpful if you could describe what you did by hand to verify
> the system and document the steps somewhere (in the ticket:741 ?) so
> that we can take the sequence as a reference to complement your patch
> with test cases .
>
Hello,
I've done some simple tests and add a brief description in the ticket.
I'll try to understand the test structure and code the tests myself
(that would be easier for everybody).
Currently on trunk I got a lot of similar mistakes when running the
test on bloodhound_search like this one:
======================================================================
ERROR: test_admin_granted_in_product_should_not_have_access (bhsearch.tests.security.MultiProductSecurityTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/user/project/bloodhound/bloodhound_search/bhsearch/tests/security.py", line 61, in setUp
self._setup_multiproduct()
File "/home/user/project/bloodhound/bloodhound_search/bhsearch/tests/security.py", line 71, in _setup_multiproduct
.upgrade_environment(self.env.db_transaction)
File "/home/user/project/bloodhound/bloodhound_multiproduct/multiproduct/api.py", line 216, in upgrade_environment
self._add_column_product_to_ticket(db)
File "/home/user/project/bloodhound/bloodhound_multiproduct/multiproduct/api.py", line 260, in _add_column_product_to_ticket
db("ALTER TABLE ticket ADD COLUMN product TEXT")
File "/home/user/project/bloodhound/bloodhound_multiproduct/multiproduct/dbcursor.py", line 120, in execute
return self.connection.execute(query, params=params)
File "/home/user/project/bloodhound/trac/trac/db/util.py", line 121, in execute
cursor.execute(query, params)
File "/home/user/project/bloodhound/bloodhound_multiproduct/multiproduct/dbcursor.py", line 85, in execute
return super(BloodhoundIterableCursor, self).execute(translate_sql(self.env, sql), args=args)
File "/home/user/project/bloodhound/trac/trac/db/util.py", line 56, in execute
r = self.cursor.execute(sql)
File "/home/user/project/bloodhound/trac/trac/db/sqlite_backend.py", line 78, in execute
result = PyFormatCursor.execute(self, *args)
File "/home/user/project/bloodhound/trac/trac/db/sqlite_backend.py", line 56, in execute
args or [])
File "/home/user/project/bloodhound/trac/trac/db/sqlite_backend.py", line 48, in _rollback_on_error
return function(self, *args, **kwargs)
OperationalError: duplicate column name: product
Does that ring a bell to someone ?
Re: Whoosh requirements
Posted by Olemis Lang <ol...@gmail.com>.
On 1/14/14, Saint Germain <sa...@gmail.com> wrote:
> On Tue, 14 Jan 2014 16:14:06 +0100, Andrej Golcov <an...@digiverse.si>
> wrote :
>
[...]
> I also intend to manually test the search features of Bloodhound in the
> next days to check if anything is broken by this Whoosh upgrade
It'd be helpful if you could describe what you did by hand to verify
the system and document the steps somewhere (in the ticket:741 ?) so
that we can take the sequence as a reference to complement your patch
with test cases .
[...]
--
Regards,
Olemis - @olemislc
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
On Tue, 14 Jan 2014 16:14:06 +0100, Andrej Golcov <an...@digiverse.si>
wrote :
> I would suggest to make Bloodhound using Whoosh 2.5 (and removing
> aforementioned workarounds) with search index rebuild when upgrading
> from setup that uses indexes created with Whoosh 2.4.
>
Hello,
I have created the ticket #741 to manage this point:
https://issues.apache.org/bloodhound/ticket/741
I added a tentative patch to remove the workaround and remove/modify
the tests accordingly.
I have no experience on Bloodhound upgrade, so I haven't touched
anything on this part.
I also intend to manually test the search features of Bloodhound in the
next days to check if anything is broken by this Whoosh upgrade (but
having read the 2.5 changelog, it seems that there are no big changes).
Best regards,
Re: Whoosh requirements
Posted by Andrej Golcov <an...@digiverse.si>.
I would suggest to make Bloodhound using Whoosh 2.5 (and removing
aforementioned workarounds) with search index rebuild when upgrading
from setup that uses indexes created with Whoosh 2.4.
Cheers, Andrej
On 14 January 2014 12:19, Anže Starič <an...@gmail.com> wrote:
> This behaviour with whoosh 2.4 is caused by WhooshEmptyFacetErrorWorkaround.
>
> Search with facets fails in whoosh 2.4.1 if the field is empty in any
> of the returned docs. A workaround was med that fills the field with
> string "empty" when the value was not provided.
>
> The same workaround is used when creating unique id-s (see
> _create_unique_id in WhooshBackend).
>
> If you disabled/removed the workaround, change in unique_id is expected.
>
> Anze
>
>
>
> 2014/1/14 Saint Germain <sa...@gmail.com>:
>> Hello !
>>
>> Yes I also noticed the api change for the score in
>> test_can_retrieve_docs. It seems to be more consistent (see
>> test_can_return_all_fields which also check to float score) than
>> before.
>>
>> I also noticed that with Whoosh 2.5.6 we have 'unique_id': u'ticket:1'
>> instead of 'unique_id': u'empty:ticket:1' in test_can_retrieve_docs.
>> It seems to be related to the Whoosh 2.4.1 fix but I am not sure.
>>
>> Best regards,
>>
>> On 14 January 2014 09:34, Anže Starič <an...@gmail.com> wrote:
>>> As soon as we change the package requirements in bloodhound_search setup.py
>>> to >5.1, new version will refuse to install if only whoosh 2.4.1 is
>>> installed, so no installations should break.
>>>
>>> As far as I can tell, Whoosh 2.4 is not supported anymore. No patches have
>>> been backported for over a year while Whoosh 2.5+ had 7 releases. I would
>>> say that bumping the required version and removing the fixes is the way to
>>> go.
>>>
>>> There is one test that fails though with Whoosh 2.5.6
>>> (WhooshBackendTestCase.test_can_retrieve_docs). It looks like an api change
>>> (score used to be unicode, but is now float), but I need to check if that
>>> is really the case.
>>>
>>>
>>> Anze
>>>
>>>
>>> 2014/1/14 Ryan Ollos <ry...@wandisco.com>
>>>
>>>> On Mon, Jan 13, 2014 at 2:44 PM, Saint Germain <sa...@gmail.com> wrote:
>>>>
>>>> > On Mon, 13 Jan 2014 11:45:40 -0800, Ryan Ollos
>>>> > <ry...@wandisco.com> wrote :
>>>> >
>>>> > > > Apparently for bloodhound_search, a specific version of Whoosh is
>>>> > > > required (2.4.1).
>>>> > > >
>>>> > > > I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
>>>> > > > apparently fix some problem in Whoosh (pull request #41).
>>>> > > >
>>>> > > > Apparently this pull request is already included in Whoosh trunk
>>>> > > > (since 2.5.1), so I was wondering if the fix is always necessary
>>>> > > > and if Whoosh version > 2.5.1 may be acceptable.
>>>> > > >
>>>> > > > Is there a thread on the topic where I can understand the problem
>>>> > > > and attempt to fix it ?
>>>> > > >
>>>> > >
>>>> > > Nothing I can remember, but you may want to search:
>>>> > > http://apache.markmail.org/search/?q=list%3Abloodhound
>>>> > >
>>>> > > The commit message associated with that file is "Added support for
>>>> > > fine-grained permissions to bhsearch", so you may want to search for
>>>> > > messages on that topic, or perhaps Anze will chime in.
>>>> > >
>>>> > > I encourage you to verifying the fixes, add additional tests if
>>>> > > needed and we can bump the version in requirements to 2.5.1 if it is
>>>> > > working well.
>>>> >
>>>> > Hello,
>>>> >
>>>> > The fix was indeed very well done. There is even a test to check that
>>>> > the fix is removed in case we don't need it anymore ! ;-)
>>>> >
>>>> > Now the question would be if we remove the fix or not ?
>>>> > I hate to break previous (and working) installation, so I'd prefer to
>>>> > keep the fix for those who want to stay with Whoosh 2.4.1.
>>>> > However if we keep it, we may end up with some spaghetti code in
>>>> > order to manage the different cases (depending on the Whoosh version).
>>>> >
>>>> > What do you advise ?
>>>> >
>>>> > Thanks for your help !
>>>> >
>>>>
>>>> I don't think we should try to support multiple major releases of Whoosh at
>>>> this time. We have enough to do without testing and debugging against
>>>> multiple versions of Whoosh.
>>>>
>>>> I guess the question is when is the right time to bump the required version
>>>> of Whoosh. That may depend on a number of issues, including at least:
>>>> features and performance of the newer versions, how long the Whoosh dev
>>>> team will continue to support the 2.4 release line with bugfixes, Python
>>>> version compatibility of the newer release .
>>>>
Re: Whoosh requirements
Posted by Anže Starič <an...@gmail.com>.
This behaviour with whoosh 2.4 is caused by WhooshEmptyFacetErrorWorkaround.
Search with facets fails in whoosh 2.4.1 if the field is empty in any
of the returned docs. A workaround was med that fills the field with
string "empty" when the value was not provided.
The same workaround is used when creating unique id-s (see
_create_unique_id in WhooshBackend).
If you disabled/removed the workaround, change in unique_id is expected.
Anze
2014/1/14 Saint Germain <sa...@gmail.com>:
> Hello !
>
> Yes I also noticed the api change for the score in
> test_can_retrieve_docs. It seems to be more consistent (see
> test_can_return_all_fields which also check to float score) than
> before.
>
> I also noticed that with Whoosh 2.5.6 we have 'unique_id': u'ticket:1'
> instead of 'unique_id': u'empty:ticket:1' in test_can_retrieve_docs.
> It seems to be related to the Whoosh 2.4.1 fix but I am not sure.
>
> Best regards,
>
> On 14 January 2014 09:34, Anže Starič <an...@gmail.com> wrote:
>> As soon as we change the package requirements in bloodhound_search setup.py
>> to >5.1, new version will refuse to install if only whoosh 2.4.1 is
>> installed, so no installations should break.
>>
>> As far as I can tell, Whoosh 2.4 is not supported anymore. No patches have
>> been backported for over a year while Whoosh 2.5+ had 7 releases. I would
>> say that bumping the required version and removing the fixes is the way to
>> go.
>>
>> There is one test that fails though with Whoosh 2.5.6
>> (WhooshBackendTestCase.test_can_retrieve_docs). It looks like an api change
>> (score used to be unicode, but is now float), but I need to check if that
>> is really the case.
>>
>>
>> Anze
>>
>>
>> 2014/1/14 Ryan Ollos <ry...@wandisco.com>
>>
>>> On Mon, Jan 13, 2014 at 2:44 PM, Saint Germain <sa...@gmail.com> wrote:
>>>
>>> > On Mon, 13 Jan 2014 11:45:40 -0800, Ryan Ollos
>>> > <ry...@wandisco.com> wrote :
>>> >
>>> > > > Apparently for bloodhound_search, a specific version of Whoosh is
>>> > > > required (2.4.1).
>>> > > >
>>> > > > I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
>>> > > > apparently fix some problem in Whoosh (pull request #41).
>>> > > >
>>> > > > Apparently this pull request is already included in Whoosh trunk
>>> > > > (since 2.5.1), so I was wondering if the fix is always necessary
>>> > > > and if Whoosh version > 2.5.1 may be acceptable.
>>> > > >
>>> > > > Is there a thread on the topic where I can understand the problem
>>> > > > and attempt to fix it ?
>>> > > >
>>> > >
>>> > > Nothing I can remember, but you may want to search:
>>> > > http://apache.markmail.org/search/?q=list%3Abloodhound
>>> > >
>>> > > The commit message associated with that file is "Added support for
>>> > > fine-grained permissions to bhsearch", so you may want to search for
>>> > > messages on that topic, or perhaps Anze will chime in.
>>> > >
>>> > > I encourage you to verifying the fixes, add additional tests if
>>> > > needed and we can bump the version in requirements to 2.5.1 if it is
>>> > > working well.
>>> >
>>> > Hello,
>>> >
>>> > The fix was indeed very well done. There is even a test to check that
>>> > the fix is removed in case we don't need it anymore ! ;-)
>>> >
>>> > Now the question would be if we remove the fix or not ?
>>> > I hate to break previous (and working) installation, so I'd prefer to
>>> > keep the fix for those who want to stay with Whoosh 2.4.1.
>>> > However if we keep it, we may end up with some spaghetti code in
>>> > order to manage the different cases (depending on the Whoosh version).
>>> >
>>> > What do you advise ?
>>> >
>>> > Thanks for your help !
>>> >
>>>
>>> I don't think we should try to support multiple major releases of Whoosh at
>>> this time. We have enough to do without testing and debugging against
>>> multiple versions of Whoosh.
>>>
>>> I guess the question is when is the right time to bump the required version
>>> of Whoosh. That may depend on a number of issues, including at least:
>>> features and performance of the newer versions, how long the Whoosh dev
>>> team will continue to support the 2.4 release line with bugfixes, Python
>>> version compatibility of the newer release .
>>>
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
Hello !
Yes I also noticed the api change for the score in
test_can_retrieve_docs. It seems to be more consistent (see
test_can_return_all_fields which also check to float score) than
before.
I also noticed that with Whoosh 2.5.6 we have 'unique_id': u'ticket:1'
instead of 'unique_id': u'empty:ticket:1' in test_can_retrieve_docs.
It seems to be related to the Whoosh 2.4.1 fix but I am not sure.
Best regards,
On 14 January 2014 09:34, Anže Starič <an...@gmail.com> wrote:
> As soon as we change the package requirements in bloodhound_search setup.py
> to >5.1, new version will refuse to install if only whoosh 2.4.1 is
> installed, so no installations should break.
>
> As far as I can tell, Whoosh 2.4 is not supported anymore. No patches have
> been backported for over a year while Whoosh 2.5+ had 7 releases. I would
> say that bumping the required version and removing the fixes is the way to
> go.
>
> There is one test that fails though with Whoosh 2.5.6
> (WhooshBackendTestCase.test_can_retrieve_docs). It looks like an api change
> (score used to be unicode, but is now float), but I need to check if that
> is really the case.
>
>
> Anze
>
>
> 2014/1/14 Ryan Ollos <ry...@wandisco.com>
>
>> On Mon, Jan 13, 2014 at 2:44 PM, Saint Germain <sa...@gmail.com> wrote:
>>
>> > On Mon, 13 Jan 2014 11:45:40 -0800, Ryan Ollos
>> > <ry...@wandisco.com> wrote :
>> >
>> > > > Apparently for bloodhound_search, a specific version of Whoosh is
>> > > > required (2.4.1).
>> > > >
>> > > > I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
>> > > > apparently fix some problem in Whoosh (pull request #41).
>> > > >
>> > > > Apparently this pull request is already included in Whoosh trunk
>> > > > (since 2.5.1), so I was wondering if the fix is always necessary
>> > > > and if Whoosh version > 2.5.1 may be acceptable.
>> > > >
>> > > > Is there a thread on the topic where I can understand the problem
>> > > > and attempt to fix it ?
>> > > >
>> > >
>> > > Nothing I can remember, but you may want to search:
>> > > http://apache.markmail.org/search/?q=list%3Abloodhound
>> > >
>> > > The commit message associated with that file is "Added support for
>> > > fine-grained permissions to bhsearch", so you may want to search for
>> > > messages on that topic, or perhaps Anze will chime in.
>> > >
>> > > I encourage you to verifying the fixes, add additional tests if
>> > > needed and we can bump the version in requirements to 2.5.1 if it is
>> > > working well.
>> >
>> > Hello,
>> >
>> > The fix was indeed very well done. There is even a test to check that
>> > the fix is removed in case we don't need it anymore ! ;-)
>> >
>> > Now the question would be if we remove the fix or not ?
>> > I hate to break previous (and working) installation, so I'd prefer to
>> > keep the fix for those who want to stay with Whoosh 2.4.1.
>> > However if we keep it, we may end up with some spaghetti code in
>> > order to manage the different cases (depending on the Whoosh version).
>> >
>> > What do you advise ?
>> >
>> > Thanks for your help !
>> >
>>
>> I don't think we should try to support multiple major releases of Whoosh at
>> this time. We have enough to do without testing and debugging against
>> multiple versions of Whoosh.
>>
>> I guess the question is when is the right time to bump the required version
>> of Whoosh. That may depend on a number of issues, including at least:
>> features and performance of the newer versions, how long the Whoosh dev
>> team will continue to support the 2.4 release line with bugfixes, Python
>> version compatibility of the newer release .
>>
Re: Whoosh requirements
Posted by Anže Starič <an...@gmail.com>.
As soon as we change the package requirements in bloodhound_search setup.py
to >5.1, new version will refuse to install if only whoosh 2.4.1 is
installed, so no installations should break.
As far as I can tell, Whoosh 2.4 is not supported anymore. No patches have
been backported for over a year while Whoosh 2.5+ had 7 releases. I would
say that bumping the required version and removing the fixes is the way to
go.
There is one test that fails though with Whoosh 2.5.6
(WhooshBackendTestCase.test_can_retrieve_docs). It looks like an api change
(score used to be unicode, but is now float), but I need to check if that
is really the case.
Anze
2014/1/14 Ryan Ollos <ry...@wandisco.com>
> On Mon, Jan 13, 2014 at 2:44 PM, Saint Germain <sa...@gmail.com> wrote:
>
> > On Mon, 13 Jan 2014 11:45:40 -0800, Ryan Ollos
> > <ry...@wandisco.com> wrote :
> >
> > > > Apparently for bloodhound_search, a specific version of Whoosh is
> > > > required (2.4.1).
> > > >
> > > > I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
> > > > apparently fix some problem in Whoosh (pull request #41).
> > > >
> > > > Apparently this pull request is already included in Whoosh trunk
> > > > (since 2.5.1), so I was wondering if the fix is always necessary
> > > > and if Whoosh version > 2.5.1 may be acceptable.
> > > >
> > > > Is there a thread on the topic where I can understand the problem
> > > > and attempt to fix it ?
> > > >
> > >
> > > Nothing I can remember, but you may want to search:
> > > http://apache.markmail.org/search/?q=list%3Abloodhound
> > >
> > > The commit message associated with that file is "Added support for
> > > fine-grained permissions to bhsearch", so you may want to search for
> > > messages on that topic, or perhaps Anze will chime in.
> > >
> > > I encourage you to verifying the fixes, add additional tests if
> > > needed and we can bump the version in requirements to 2.5.1 if it is
> > > working well.
> >
> > Hello,
> >
> > The fix was indeed very well done. There is even a test to check that
> > the fix is removed in case we don't need it anymore ! ;-)
> >
> > Now the question would be if we remove the fix or not ?
> > I hate to break previous (and working) installation, so I'd prefer to
> > keep the fix for those who want to stay with Whoosh 2.4.1.
> > However if we keep it, we may end up with some spaghetti code in
> > order to manage the different cases (depending on the Whoosh version).
> >
> > What do you advise ?
> >
> > Thanks for your help !
> >
>
> I don't think we should try to support multiple major releases of Whoosh at
> this time. We have enough to do without testing and debugging against
> multiple versions of Whoosh.
>
> I guess the question is when is the right time to bump the required version
> of Whoosh. That may depend on a number of issues, including at least:
> features and performance of the newer versions, how long the Whoosh dev
> team will continue to support the 2.4 release line with bugfixes, Python
> version compatibility of the newer release .
>
Re: Whoosh requirements
Posted by Ryan Ollos <ry...@wandisco.com>.
On Mon, Jan 13, 2014 at 2:44 PM, Saint Germain <sa...@gmail.com> wrote:
> On Mon, 13 Jan 2014 11:45:40 -0800, Ryan Ollos
> <ry...@wandisco.com> wrote :
>
> > > Apparently for bloodhound_search, a specific version of Whoosh is
> > > required (2.4.1).
> > >
> > > I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
> > > apparently fix some problem in Whoosh (pull request #41).
> > >
> > > Apparently this pull request is already included in Whoosh trunk
> > > (since 2.5.1), so I was wondering if the fix is always necessary
> > > and if Whoosh version > 2.5.1 may be acceptable.
> > >
> > > Is there a thread on the topic where I can understand the problem
> > > and attempt to fix it ?
> > >
> >
> > Nothing I can remember, but you may want to search:
> > http://apache.markmail.org/search/?q=list%3Abloodhound
> >
> > The commit message associated with that file is "Added support for
> > fine-grained permissions to bhsearch", so you may want to search for
> > messages on that topic, or perhaps Anze will chime in.
> >
> > I encourage you to verifying the fixes, add additional tests if
> > needed and we can bump the version in requirements to 2.5.1 if it is
> > working well.
>
> Hello,
>
> The fix was indeed very well done. There is even a test to check that
> the fix is removed in case we don't need it anymore ! ;-)
>
> Now the question would be if we remove the fix or not ?
> I hate to break previous (and working) installation, so I'd prefer to
> keep the fix for those who want to stay with Whoosh 2.4.1.
> However if we keep it, we may end up with some spaghetti code in
> order to manage the different cases (depending on the Whoosh version).
>
> What do you advise ?
>
> Thanks for your help !
>
I don't think we should try to support multiple major releases of Whoosh at
this time. We have enough to do without testing and debugging against
multiple versions of Whoosh.
I guess the question is when is the right time to bump the required version
of Whoosh. That may depend on a number of issues, including at least:
features and performance of the newer versions, how long the Whoosh dev
team will continue to support the 2.4 release line with bugfixes, Python
version compatibility of the newer release .
Re: Whoosh requirements
Posted by Saint Germain <sa...@gmail.com>.
On Mon, 13 Jan 2014 11:45:40 -0800, Ryan Ollos
<ry...@wandisco.com> wrote :
> > Apparently for bloodhound_search, a specific version of Whoosh is
> > required (2.4.1).
> >
> > I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
> > apparently fix some problem in Whoosh (pull request #41).
> >
> > Apparently this pull request is already included in Whoosh trunk
> > (since 2.5.1), so I was wondering if the fix is always necessary
> > and if Whoosh version > 2.5.1 may be acceptable.
> >
> > Is there a thread on the topic where I can understand the problem
> > and attempt to fix it ?
> >
>
> Nothing I can remember, but you may want to search:
> http://apache.markmail.org/search/?q=list%3Abloodhound
>
> The commit message associated with that file is "Added support for
> fine-grained permissions to bhsearch", so you may want to search for
> messages on that topic, or perhaps Anze will chime in.
>
> I encourage you to verifying the fixes, add additional tests if
> needed and we can bump the version in requirements to 2.5.1 if it is
> working well.
Hello,
The fix was indeed very well done. There is even a test to check that
the fix is removed in case we don't need it anymore ! ;-)
Now the question would be if we remove the fix or not ?
I hate to break previous (and working) installation, so I'd prefer to
keep the fix for those who want to stay with Whoosh 2.4.1.
However if we keep it, we may end up with some spaghetti code in
order to manage the different cases (depending on the Whoosh version).
What do you advise ?
Thanks for your help !
Re: Whoosh requirements
Posted by Ryan Ollos <ry...@wandisco.com>.
On Mon, Jan 13, 2014 at 10:17 AM, Saint Germain <sa...@gmail.com> wrote:
> Hello,
>
> Apparently for bloodhound_search, a specific version of Whoosh is
> required (2.4.1).
>
> I noticed the file bloodhound_search/bhsearch/whoosh_fixes.py which
> apparently fix some problem in Whoosh (pull request #41).
>
> Apparently this pull request is already included in Whoosh trunk
> (since 2.5.1), so I was wondering if the fix is always necessary and if
> Whoosh version > 2.5.1 may be acceptable.
>
> Is there a thread on the topic where I can understand the problem and
> attempt to fix it ?
>
Nothing I can remember, but you may want to search:
http://apache.markmail.org/search/?q=list%3Abloodhound
The commit message associated with that file is "Added support for
fine-grained permissions to bhsearch", so you may want to search for
messages on that topic, or perhaps Anze will chime in.
I encourage you to verifying the fixes, add additional tests if needed and
we can bump the version in requirements to 2.5.1 if it is working well.