[Box Backup-dev] COMMIT r235 - box/chris/boxi/bin/bbstored

[Ben Summers]

On 15 Dec 2005, at 13:07, Chris Wilson wrote:

> Hi Ben,
>
> Thanks for looking at this patch.
>
>> I don't this is quite right.
>
> It probably isn't, after all I wrote it :-)

Hmmm.

>
>> The name of error is wrong. Why would the client care about  
>> RaidFiles anyway? Surely the error should be "directory doesn't  
>> exist".
>>
>> What problem are you attempting to solve here?
>
> When the store is corrupt on the server (e.g. missing raidfile  
> directory), bbstored will die when trying to read it, which gives  
> the client a TLSReadFailed. I couldn't think of a less helpful  
> error to send to the client, so I tried to improve it.
>
> I wanted the client to be able to phone/email the server operator  
> and say something more useful than "it doesn't work" or "the server  
> disconnected me". Perhaps they shouldn't care about RAID files, but  
> the error should at least indicate that the store is corrupt and  
> the server admin should run bbstoreaccounts check on it.

Surely the server admin will be monitoring their server? If not, then  
you should not be trusting them anyway.

In most cases, that error will not happen because of corrupt stores.  
Do you really want the client to think their backups are corrupt when  
most likely they're not?

>
>> In which case, checking for a RaidFile exception isn't quite  
>> right, because you're missing the case where the object ID refers  
>> to a file.
>
> Sorry, what case? Surely that's the client's business. It always  
> asks the server for an object ID.

An object can be a file or a directory. If you try to use a file  
object ID where a directory is expected, you'll get a different error.

And surely the fact that it's using a RaidFile is an implementation  
detail which the client has no need to know, and in fact, just  
obscures the real problem?

>
>> Also, brace placement is against style guide.
>
> Sorry, which one, the try/catch?
>
>>>  +	try {
> [...]
>>>  +	} catch (RaidFileException &e) {

That's the one.

	try
	{
		...
	}
	catch(...)
	{
		...
	}

Ben