[ditrack commit] r1486 - in src/trunk: . DITrack DITrack/Command DITrack/Util DITrack/dt

[Ivan Glushkov]

Hi.

On 4/9/07, Vlad Skvortsov <vss at 73rus.com> wrote:
> Hi!
>
> I'm sure this commit does what it needed, but I have a few concerns
> about it. First, as I've said earlier, I'm not in favor of idea of
> mixing environment variables with DITrack-specific variables. Thus my
> suggestion is to have clear distinction (by means of syntax) between them:
>
> $VAR == environment variable
> $DT:VAR == DITrack-specific variable (that *may* happen to have the same
> value as an environment variable).
What do you mean 'mixing environment variables with DITrack-specific
variables'.  They are mixed if dt-variables don't begin with "DT:"?


> 'globals' is way too dt (command line client) specific thing to make use
> of it in a database module which may be reused by different kinds of
> clients. I suggest passing a dictionary instead, e.g.:
>
> vars = dict(os.environ)
> vars["DT:USER"] = fetch_user_name_from_wherever()
>
> f = DITrack.DB.Filter(e, vars)

agree.

> > -        globals.get_username(opts, db)
> > +        globals.get_username(opts, db.cfg.users)
> >
>
> Hm, again - db.cfg.users is way too implementation-dependent to hardcode
> it in every single place where we need to figure out the current user
> name. I'd strongly prefer passing database object instead (thus if we
> need to change it, we don't have to chase for it throughout all the code).
>
> [skipped]




===================================================================
> > --- src/trunk/DITrack/dt/globals.py   2007-03-22 09:02:50 UTC (rev 1485)
> > +++ src/trunk/DITrack/dt/globals.py   2007-03-23 15:07:58 UTC (rev 1486)
> > @@ -119,7 +119,7 @@
> >              if not (st[stat.ST_MODE] & stat.S_IEXEC):
> >                  DITrack.Util.common.err(msg + "is not executable")
> >
> > -    def get_username(self, opts, db):
> > +    def get_username(self, opts, correct_users=None):
> >
>
> Why not something like this instead:
>
> def get_username(self, opts, db, valid_users_only=True)
>
> ?
>
> It's 'globals' that should depend on 'Database', not the other way around.

To correctly form filters while creating Database() we should know
username (as filters refer to the ditrack username).
So, we should determine it with 'globals.get_username()' before
'open_db()'(or 'Database()') and to pass it there in 'globals' object.
username belongs only to 'globals' object, doesn't it?
So, database depends on 'globals' and not the reverse

Frankly speaking, we don't need database object at all to determine
what the username is in the environment, do we?
We only should know about 'os.environ' and 'opts.var'. So why should
we pass db to 'get_username()'?

So i changed 'get_username()' as it wanted 'db' as parameter. So we couldn't do:

globals.get_username(opts,db)
db = open_db(globals,opt)


> Keep going :-)
:)

Ivan