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

[Ivan Glushkov]

On Wed, 18 Apr 2007 00:16:17 +0400, Vlad Skvortsov <vss at 73rus.com> wrote:

> Ivan Glushkov wrote:
>>> 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:"?
>
> Yes. By looking at a variable name a user should be able to infer where  
> the data comes from.
>
>
> [skipped]
>
>>> > -    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
>
> I meant that 'Database' has much broader context than 'globals'.  
> Database could be used by arbitrary number of clients, which should not  
> be obliged to instantiate 'globals' object similar to that in 'dt'.
>
>>
>> Frankly speaking, we don't need database object at all to determine
>> what the username is in the environment, do we?
>
> Correct. Though we need the Database to determine if the username is  
> valid.
>
>> We only should know about 'os.environ' and 'opts.var'. So why should
>> we pass db to 'get_username()'?
>
> Historically we did this to check the validity of the username right  
> away.
>
>>
>> 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)
>
> Hmm. I guess what we need to do is:
>
> 1) make get_username() do just one thing -- figuring out the username;
> 2) pass raw username to the Database constructor;
> 3) pass open mode (readonly/modify) to the Database constructor.
>
> In the readonly mode, Database() shouldn't check for the username  
> validity. Otherwise it should and will raise an exception if the name  
> passed is not valid.
>
> I think it's along the lines with something you've proposed in one of  
> the emails. Comments?

no-comments.
i agree.