incubator-bloodhound-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Olemis Lang <ole...@gmail.com>
Subject Re: [BEP-0003] Request for comments : DB configuration vs ConfigParser
Date Mon, 07 Jan 2013 04:36:53 GMT
On 1/4/13, Olemis Lang <olemis@gmail.com> wrote:
>
[...]
>
> ... so I'd like to know beforehand if anybody has anything to say
> about that subject . Comments ? Objections ?
>

A few more comments , based on my recent experience building a solution for #115

  - I noticed that `ModelBase.update` method invokes
    `TicketSystem.reset_ticket_fields` inside a
    DB transaction. I wander if this is still needed for models
    not related to tickets at all (e.g. product settings) .
  - It'd be nice to modify the signature (or provide an alias)
    for `ModelBase.__init__` in order to be able to invoke it
    using keyword arguments i.e. Model(env, {x=... , y=...})
    will be Model(env, x=... , y=...)
    * franco had the idea and started to do a similar thing
      ( https://bitbucket.org/jose_angel_franco/bloodhound/commits/4247c62b27e92995c8f37008d0b7989b#Lbloodhound_dashboard/bhdashboard/model.pyT55
)
      so I guess he might be able to provide one such solution
      in the near future .
  - We should add batch operations in ModelBase . That includes
    batch inserts , batch updates and removal of multiple DB records.
  - Some modules make use of `with` statements without __future__ import
  - Current implementation of `ModelBase.delete` seems not to offer
    the possibility of executing these modifications in the context of
    an existing DB transaction . IOW , `ModelBase.select` includes `db`
    parameter but `ModelBase.delete` doesn't . Should it be added ?
    AFAICT the same happens with `ModelBase.insert` , etc ...
  - In `ModelBase.__setattr__` there is a snippet shown below . However
    in that case self is bound to an instance of `ModelBase` direct
    object descendant . Execution of else branch should raise a type error
    isn't it ? Besides I don't understand what is  it for .

  {{{
  #!python

    if data and fields and name in fields:
        self._data[name] = value
    else:
        dict.__setattr__(self, name, value)

  }}}

  As a matter of fact it seems to me this might be the cause why these
  lines of code do not work as they are supposed to , unless I'm
  missing something . In that case setting.value is always None , even
  after attribute assignment .

  {{{
  #!python

  class ProductSetting(ModelBase):
      ...

  setting = ProductSetting(env)
  setting._data.update(key)
  setting.value = value
  setting.insert()
  }}}

  However , maybe it's just that setting.data is empty ... I'm not sure .

If some of these deserve new tickets I'll move forward and
create some.

-- 
Regards,

Olemis.

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

Featured article:

Mime
View raw message