[MERGE][0.8] break-lock command
Martin Pool
mbp at sourcefrog.net
Fri May 5 06:46:50 BST 2006
On 05/05/2006, at 12:47 PM, Robert Collins wrote:
>> Given the danger of cStringIO with Unicode string I'd like to make
>> some
>> effort in the future to remove or reduce use of it. Perhaps we
>> could go
>> through a decorator class that will prevent any attempt to put
>> Unicode
>> in.
>
> mmm, sure I guess. Is this a general comment or a request that the
> patch
> be changed ?
It's a general comment; I've added it to my list and you needn't do
anything now.
>> Shouldn't this be calling the super implementation?
>>
>> To avoid this kind of bug I'd rather have call addCleanup from the
>> setUp method - it puts Same Things Together.
>
> Meep, yes it should. addCleanup still doesn't fit in my head in casual
> programming, I need to look at why.
Let's try to work out why, or what would be a better fit.
>>> + def test_unlocked_repo_locked(self):
>>> + # break lock on the branch should try on the repository
>>> even
>>> + # if the branch isn't locked
>>> + self.branch.repository.lock_write()
>>> + bzrlib.ui.ui_factory.stdin = StringIO("y\n")
>>> + try:
>>> + self.unused_branch.break_lock()
>>> + except NotImplementedError:
>>> + # branch does not support break_lock
>>> + self.branch.repository.unlock()
>>> + return
>>> + self.assertRaises(errors.LockBroken,
>>> self.branch.repository.unlock)
>>
>> q: would it be better to say 'skipped' if it's unimplemented, rather
>> than just return?
>
> No: Being unimplemented is valid for this interface : it means that
> the
> branch does not suffer from broken locks. . There is nothing 'Wrong' -
> and our definition for 'skipped' is 'There is something you could
> do to
> fix this.' Constrast with format 4 branches, where we can fix it by
> restoring the format 4 write logic.
General discussion:
But as John said, isn't it possible that the sftp-only locks used by
old formats could be left behind?
Also we might want to distinguish "This is not implemented yet" from
"This is not supported by this format". I think of
NotImplementedError as more of the former.
>
> (Can I call again for the nuking with extrem prejuidice of the SFTP
> connection cache ?)
Yes, please do. It seems to me that using a single sftp connection
for all operations needed for a user-level command is not just an
optimisation but a hard requirement: the user should not be required
to enter their passphrase twice just because it happens to fall out
of the cache. Therefore, we should actually keep around a Transport
that holds the reference.
--
Martin
More information about the bazaar
mailing list