You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bloodhound.apache.org by Olemis Lang <ol...@gmail.com> on 2012/12/20 22:26:05 UTC

Re: [Apache Bloodhound] #115: Product-specific settings

On 12/19/12, Apache Bloodhound <bl...@incubator.apache.org> wrote:
>
>  I'll be developing this in [https://bitbucket.org/olemis/bloodhound-mq my
>  patch queue @ Bitbucket] in [https://bitbucket.org/olemis/bloodhound-
>  mq/commits/all/tip/branch(%22t115_product_env%22) branch t115_product_env]
>

Today I have submitted new versions of these patches (in branch
t115_product_env ) aimed at running tests for Trac environments
against product environments .

Important observations :

  - Global environment's config object is reused for the moment.
    #115 is exactly about adding product-specific settings so I'll
    move forward that way .
  - NotImplementedError raised for DB methods . The reason for
    doing this is that these methods (i.e. get_db_cnx ,
    with_transaction , get_read_db , db_query , db_transaction)
    are the exact locations where jure's SQL proxies should be
    installed , so I leave that to Jure ;)
  - Product environment will not participate in environment
    setup / upgrade because global environment is the one
    responsible for such tasks . Setup participants list will
    always be empty in product scope . Further enhancements
    needed in there though , especially when deciding what
    components will be enabled / disabled .

Many enhancements and tests needed . Consider this an early preview
with some initial testing code in place ;)

BTW

{{{
#!sh

$ /path/to/python /path/to/bloodhound_multiproduct/tests/env.py
E.
======================================================================
ERROR: Testing env.get_known_users
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/path/to/trac/tests/env.py", line 30, in test_get_known_users
    with self.env.db_transaction as db:
  File "/path/to/bloodhound_multiproduct/multiproduct/env.py", line
342, in db_transaction
    raise NotImplementedError
NotImplementedError

----------------------------------------------------------------------
Ran 2 tests in 6.919s

FAILED (errors=1)

}}}

-- 
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Olemis Lang <ol...@gmail.com>.
On 1/13/13, Jure Zitnik <ju...@digiverse.si> wrote:
> On 1/13/13 7:07 AM, Olemis Lang wrote:
>> On 1/11/13, Jure Zitnik <ju...@digiverse.si> wrote:
>>> On 1/10/13 4:15 AM, Olemis Lang wrote:
>>>
>> [...]
>>>>> I started integrating ProductEnvironment with the database proxy
>>>>> (#288),
>>>> good .
>>>>
>>>>> environment factory (#322)
>>>> I'll take a look to see what's been done about this ... is there
>>>> anything already in svn repos ?
>>>>
>>>>> and global hooks (#323).
>>>>>
>>>> :)
>>> All the work done in integrating #115 with #288 and work done on #322
>>> and #323 is in the svn repo on bep_0003_multiproduct branch.
>>>
>> Is this usable your side ? I ask because
>>
[...]
>>
>> "/path/to/bloodhoundbloodhound_multiproduct/multiproduct/model.py",
>> line 88, in insert
>>      from trac.bloodhound import  TRANSLATE_TABLES
>> ImportError: No module named bloodhound
>>
>> ----------------------------------------------------------------------
>> Ran 2 tests in 8.896s
>>
>> FAILED (errors=2)
>>
[...]
> Missed that one when refactoring the code, r1432576 should fix that.

ok thnx

> Even after this tests are still failing in env.get_known_users with
> IntegrityError: column name is not unique ...

ok . Even if that's the case , now if I write new test cases then I
should be able to run the test suite ... and build patches against
bep_0003_multiproduct branch .

;)

[...]

-- 
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Jure Zitnik <ju...@digiverse.si>.
On 1/13/13 7:07 AM, Olemis Lang wrote:
> On 1/11/13, Jure Zitnik <ju...@digiverse.si> wrote:
>> On 1/10/13 4:15 AM, Olemis Lang wrote:
>>
> [...]
>>>> I started integrating ProductEnvironment with the database proxy (#288),
>>> good .
>>>
>>>> environment factory (#322)
>>> I'll take a look to see what's been done about this ... is there
>>> anything already in svn repos ?
>>>
>>>> and global hooks (#323).
>>>>
>>> :)
>> All the work done in integrating #115 with #288 and work done on #322
>> and #323 is in the svn repo on bep_0003_multiproduct branch.
>>
> Is this usable your side ? I ask because
>
> {{{
> #!sh
>
> $ python tests/env.py
> EE
> ======================================================================
> ERROR: Testing env.get_known_users
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>    File "tests/env.py", line 117, in setUp
>      self._load_product_from_data(self.global_env, self.default_product)
>    File "tests/env.py", line 91, in _load_product_from_data
>      product.insert()
>    File "/path/to/bloodhoundbloodhound_multiproduct/multiproduct/model.py",
> line 88, in insert
>      from trac.bloodhound import  TRANSLATE_TABLES
> ImportError: No module named bloodhound
>
> ======================================================================
> ERROR: Testing env.get_version
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>    File "tests/env.py", line 117, in setUp
>      self._load_product_from_data(self.global_env, self.default_product)
>    File "tests/env.py", line 91, in _load_product_from_data
>      product.insert()
>    File "/path/to/bloodhoundbloodhound_multiproduct/multiproduct/model.py",
> line 88, in insert
>      from trac.bloodhound import  TRANSLATE_TABLES
> ImportError: No module named bloodhound
>
> ----------------------------------------------------------------------
> Ran 2 tests in 8.896s
>
> FAILED (errors=2)
>
> $ hg log
Missed that one when refactoring the code, r1432576 should fix that. 
Even after this tests are still failing in env.get_known_users with 
IntegrityError: column name is not unique ... Will take a deeper look as 
this is probably connected with #288 ...

Cheers,
Jure



Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Olemis Lang <ol...@gmail.com>.
On 1/11/13, Jure Zitnik <ju...@digiverse.si> wrote:
> On 1/10/13 4:15 AM, Olemis Lang wrote:
>
[...]
>>> I started integrating ProductEnvironment with the database proxy (#288),
>> good .
>>
>>> environment factory (#322)
>> I'll take a look to see what's been done about this ... is there
>> anything already in svn repos ?
>>
>>> and global hooks (#323).
>>>
>> :)
>
> All the work done in integrating #115 with #288 and work done on #322
> and #323 is in the svn repo on bep_0003_multiproduct branch.
>

Is this usable your side ? I ask because

{{{
#!sh

$ python tests/env.py
EE
======================================================================
ERROR: Testing env.get_known_users
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/env.py", line 117, in setUp
    self._load_product_from_data(self.global_env, self.default_product)
  File "tests/env.py", line 91, in _load_product_from_data
    product.insert()
  File "/path/to/bloodhoundbloodhound_multiproduct/multiproduct/model.py",
line 88, in insert
    from trac.bloodhound import  TRANSLATE_TABLES
ImportError: No module named bloodhound

======================================================================
ERROR: Testing env.get_version
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tests/env.py", line 117, in setUp
    self._load_product_from_data(self.global_env, self.default_product)
  File "tests/env.py", line 91, in _load_product_from_data
    product.insert()
  File "/path/to/bloodhoundbloodhound_multiproduct/multiproduct/model.py",
line 88, in insert
    from trac.bloodhound import  TRANSLATE_TABLES
ImportError: No module named bloodhound

----------------------------------------------------------------------
Ran 2 tests in 8.896s

FAILED (errors=2)

$ hg log -r . --template="{svnrev}\n"
1432088


}}}

o.O

[...]

-- 
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Jure Zitnik <ju...@digiverse.si>.
On 1/10/13 4:15 AM, Olemis Lang wrote:
>> I commited patches in r1430768. 
> I see . I'll try to explain something we've being doing since long
> time ago (based on Gary's suggestions) and I think is very helpful .
>
> Commits should be focused considering their intent . The more focused
> the better . That's exactly why I developed three patches for
> (related) features : 1. product environments , 2. product config , 3.
> test code . Therefore each patch should be applied in a separate
> changeset (e.g. like in #146 [1]_ )
>
> You might say «that's insane» ... but take as example #341 [2]_ which
> is a regression after #146 . As I've also followed the same approach
> to build the patches I submit , it's possible to identify the exact
> point [3]_ where this very same issue was fixed once upon a time (...
> so let's all hail Gary ! )

Ok, thanks for the explanation, sounds reasonable, will follow these 
guidelines in the future ;)

>> I started integrating ProductEnvironment with the database proxy (#288),
> good .
>
>> environment factory (#322)
> I'll take a look to see what's been done about this ... is there
> anything already in svn repos ?
>
>> and global hooks (#323).
>>
> :)

All the work done in integrating #115 with #288 and work done on #322 
and #323 is in the svn repo on bep_0003_multiproduct branch.

>> A question re ProductEnvironment class - is there a specific reason for
>> that class not being derived directly from trac.env.Environment?
> We've been talking about this via #bloodhound @ freenode . Could you
> please post a summary ?

Short summary would be that even though trac.env.Environment and 
ProductEnvironment share the same interface they are semantically 
different. That and because the intent is to keep the product 
environments transient (lightweight) they're implemented as a separate 
class (and not derived from trac.env.Environment). Only methods/props 
that require special handling are implemented/overriden in 
ProductEnvironment, the rest is forwarded to the global 
trac.env.Environment.

>> I'm
>> asking this as I was planning on replacing (monkey patching)
>> trac.env.Environment with our implementation of the environment. This
>> way we have control over database connection in all environments (global
>> and per product).
> I've mentioned this before and mentioned that the way to go should be
> to install product-specific DB proxies in each instance of
> ProductEnvironment class . In order to do that (and in general)
> there's no need to extend Environment class at all . ProductEnvs are
> wrappers .
>
>> Making that (our env) a base class for
>> ProductEnvironment would make things more consistent.
>>
> I don't think so .

Agree now that some more light has been shed on the ProductEnvironments :)

Cheers,
Jure


Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Olemis Lang <ol...@gmail.com>.
On 1/9/13, Jure Zitnik <ju...@digiverse.si> wrote:
> On 1/8/13 9:07 AM, Olemis Lang wrote:
>> On 12/21/12, Jure Zitnik <ju...@digiverse.si> wrote:
>>> On 12/20/12 10:26 PM, Olemis Lang wrote:
>>>> On 12/19/12, Apache Bloodhound <bl...@incubator.apache.org>
>>>> wrote:
>>>>>    I'll be developing this in
>>>>> [https://bitbucket.org/olemis/bloodhound-mq
>>>>> my
>>>>>    patch queue @ Bitbucket] in
>>>>> [https://bitbucket.org/olemis/bloodhound-
>>>>>    mq/commits/all/tip/branch(%22t115_product_env%22) branch
>>>>> t115_product_env]
>>>>>
>>>> Today I have submitted new versions of these patches (in branch
>>>> t115_product_env ) aimed at running tests for Trac environments
>>>> against product environments .
>>> Great, will take a look.
>> it's done now , so enjoy !
>> ;)
>
> I commited patches in r1430768.
>

I see . I'll try to explain something we've being doing since long
time ago (based on Gary's suggestions) and I think is very helpful .

Commits should be focused considering their intent . The more focused
the better . That's exactly why I developed three patches for
(related) features : 1. product environments , 2. product config , 3.
test code . Therefore each patch should be applied in a separate
changeset (e.g. like in #146 [1]_ )

You might say «that's insane» ... but take as example #341 [2]_ which
is a regression after #146 . As I've also followed the same approach
to build the patches I submit , it's possible to identify the exact
point [3]_ where this very same issue was fixed once upon a time (...
so let's all hail Gary ! )

> I started integrating ProductEnvironment with the database proxy (#288),

good .

> environment factory (#322)

I'll take a look to see what's been done about this ... is there
anything already in svn repos ?

> and global hooks (#323).
>

:)

> A question re ProductEnvironment class - is there a specific reason for
> that class not being derived directly from trac.env.Environment?

We've been talking about this via #bloodhound @ freenode . Could you
please post a summary ?

> I'm
> asking this as I was planning on replacing (monkey patching)
> trac.env.Environment with our implementation of the environment. This
> way we have control over database connection in all environments (global
> and per product).

I've mentioned this before and mentioned that the way to go should be
to install product-specific DB proxies in each instance of
ProductEnvironment class . In order to do that (and in general)
there's no need to extend Environment class at all . ProductEnvs are
wrappers .

> Making that (our env) a base class for
> ProductEnvironment would make things more consistent.
>

I don't think so .

> Also, I was thinking of renaming ProductEnvironment.env to
> ProductEnvironment.parent or ProductEnvironment.global (as set forth in
> BEP-0003). What's your take on this, do you have any objections?
>

about attribute name ? not really ...

>>> any chance of rebasing those patches
>>> against the multiproduct branch (bep_0003_multiproduct)?
>> of course . IMO they should work with no or little modifications ...
>> especially depending on base version (common ancestor) of
>> bep_0003_multiproduct with respect to trunk . Do you merge changes in
>> trunk relatively often ?
>
> Thanks for the rebase. Until yesterday the trunk merge frequency was
> rather low but is set to increase ;)
>

wise words

>
>> so it is the other way round . What's the next step for multi-product
>> support ? What shall I do ?
>
> Next step for the multi-product is the integration I'm working on at the
> moment (mentioned above). Depending on how this goes we'll see where to
> head next ...

Well for the time being I'll follow with product-specific component
rules , which does not require DB access but configuration fu .

> there are already number of tickets re UI changes related
> to multi-product (324-329) so that'd be the most obvious way.
>

maybe later ... ;)

.. [1] Ticket #146
        (https://issues.apache.org/bloodhound/ticket/146#comment:41)

.. [2] Ticket #341
        (https://issues.apache.org/bloodhound/ticket/341#comment:4)

.. [3] BH Theme #146 : Workflow drop down visible on affix
        (https://bitbucket.org/olemis/bloodhound_theme-mq/commits/68a6509eb10c258b16cf7a287dace19c1bfcaa4b)

-- 
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Jure Zitnik <ju...@digiverse.si>.
On 1/8/13 9:07 AM, Olemis Lang wrote:
> On 12/21/12, Jure Zitnik <ju...@digiverse.si> wrote:
>> On 12/20/12 10:26 PM, Olemis Lang wrote:
>>> On 12/19/12, Apache Bloodhound <bl...@incubator.apache.org>
>>> wrote:
>>>>    I'll be developing this in [https://bitbucket.org/olemis/bloodhound-mq
>>>> my
>>>>    patch queue @ Bitbucket] in [https://bitbucket.org/olemis/bloodhound-
>>>>    mq/commits/all/tip/branch(%22t115_product_env%22) branch
>>>> t115_product_env]
>>>>
>>> Today I have submitted new versions of these patches (in branch
>>> t115_product_env ) aimed at running tests for Trac environments
>>> against product environments .
>> Great, will take a look.
> it's done now , so enjoy !
> ;)

I commited patches in r1430768.

I started integrating ProductEnvironment with the database proxy (#288), 
environment factory (#322) and global hooks (#323).

A question re ProductEnvironment class - is there a specific reason for 
that class not being derived directly from trac.env.Environment? I'm 
asking this as I was planning on replacing (monkey patching) 
trac.env.Environment with our implementation of the environment. This 
way we have control over database connection in all environments (global 
and per product). Making that (our env) a base class for 
ProductEnvironment would make things more consistent.

Also, I was thinking of renaming ProductEnvironment.env to 
ProductEnvironment.parent or ProductEnvironment.global (as set forth in 
BEP-0003). What's your take on this, do you have any objections?

>> any chance of rebasing those patches
>> against the multiproduct branch (bep_0003_multiproduct)?
> of course . IMO they should work with no or little modifications ...
> especially depending on base version (common ancestor) of
> bep_0003_multiproduct with respect to trunk . Do you merge changes in
> trunk relatively often ?

Thanks for the rebase. Until yesterday the trunk merge frequency was 
rather low but is set to increase ;)


> so it is the other way round . What's the next step for multi-product
> support ? What shall I do ?

Next step for the multi-product is the integration I'm working on at the 
moment (mentioned above). Depending on how this goes we'll see where to 
head next ... there are already number of tickets re UI changes related 
to multi-product (324-329) so that'd be the most obvious way.

Cheers,
Jure



Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Olemis Lang <ol...@gmail.com>.
On 12/21/12, Jure Zitnik <ju...@digiverse.si> wrote:
> Hi Olemis,
>

:)

> On 12/20/12 10:26 PM, Olemis Lang wrote:
>> On 12/19/12, Apache Bloodhound <bl...@incubator.apache.org>
>> wrote:
>>>   I'll be developing this in [https://bitbucket.org/olemis/bloodhound-mq
>>> my
>>>   patch queue @ Bitbucket] in [https://bitbucket.org/olemis/bloodhound-
>>>   mq/commits/all/tip/branch(%22t115_product_env%22) branch
>>> t115_product_env]
>>>
>> Today I have submitted new versions of these patches (in branch
>> t115_product_env ) aimed at running tests for Trac environments
>> against product environments .
>
> Great, will take a look.

it's done now , so enjoy !
;)

> One observation though - I noticed that the
> patches are against the trunk,

yep

> any chance of rebasing those patches
> against the multiproduct branch (bep_0003_multiproduct)?

of course . IMO they should work with no or little modifications ...
especially depending on base version (common ancestor) of
bep_0003_multiproduct with respect to trunk . Do you merge changes in
trunk relatively often ?

> Would make the
> patch process much easier.

I think recent changes in trunk should be merged with
bep_0003_multiproduct branch . You'll have to do so in the end I guess
, and there will be some important enhancements for DB models , #333 ,
etc ...

This is what I'll do , I'll create a new branch @ the aforementioned
patch queue repository to perform such rebase *if* that's the way to
go .

>> Important observations :
>>
>>    - Global environment's config object is reused for the moment.
>>      #115 is exactly about adding product-specific settings so I'll
>>      move forward that way .

This is all wrong . I discarded environment inheritance by default to
make Configuration objects more compatible with Trac's

>>    - NotImplementedError raised for DB methods .
[...]
>
> ;)
>

jftr , this is still true .

>>    - Product environment will not participate in environment
>>      setup / upgrade
>>
[...]

this is still the case , now documented in BEP 3 .

>> Many enhancements and tests needed . Consider this an early preview
>> with some initial testing code in place ;)
>
> If there's something I can help you with (re #115) just let me know, I
> can pick that up after getting the SQL proxy integrated.
>

now it's done !
;)

so it is the other way round . What's the next step for multi-product
support ? What shall I do ?

I'll check related open tickets too ... later today .

-- 
Regards,

Olemis.

Blog ES: http://simelo-es.blogspot.com/
Blog EN: http://simelo-en.blogspot.com/

Featured article:

Re: [Apache Bloodhound] #115: Product-specific settings

Posted by Jure Zitnik <ju...@digiverse.si>.
Hi Olemis,

On 12/20/12 10:26 PM, Olemis Lang wrote:
> On 12/19/12, Apache Bloodhound <bl...@incubator.apache.org> wrote:
>>   I'll be developing this in [https://bitbucket.org/olemis/bloodhound-mq my
>>   patch queue @ Bitbucket] in [https://bitbucket.org/olemis/bloodhound-
>>   mq/commits/all/tip/branch(%22t115_product_env%22) branch t115_product_env]
>>
> Today I have submitted new versions of these patches (in branch
> t115_product_env ) aimed at running tests for Trac environments
> against product environments .

Great, will take a look. One observation though - I noticed that the 
patches are against the trunk, any chance of rebasing those patches 
against the multiproduct branch (bep_0003_multiproduct)? Would make the 
patch process much easier.
> Important observations :
>
>    - Global environment's config object is reused for the moment.
>      #115 is exactly about adding product-specific settings so I'll
>      move forward that way .
>    - NotImplementedError raised for DB methods . The reason for
>      doing this is that these methods (i.e. get_db_cnx ,
>      with_transaction , get_read_db , db_query , db_transaction)
>      are the exact locations where jure's SQL proxies should be
>      installed , so I leave that to Jure ;)

;)

>    - Product environment will not participate in environment
>      setup / upgrade because global environment is the one
>      responsible for such tasks . Setup participants list will
>      always be empty in product scope . Further enhancements
>      needed in there though , especially when deciding what
>      components will be enabled / disabled .
>
> Many enhancements and tests needed . Consider this an early preview
> with some initial testing code in place ;)

If there's something I can help you with (re #115) just let me know, I 
can pick that up after getting the SQL proxy integrated.

Cheers,
Jure