blazer icon indicating copy to clipboard operation
blazer copied to clipboard

Redundant API calls in Object.Attrs, elsewhere

Open kurin opened this issue 7 years ago • 7 comments

restic/restic#1383 is showing a lot of class B transactions.

Restic calls Attrs to ensure that an object doesn't exist before uploading it. This calls object.ensure, which populates the object ID and name, and then (on objects that exist) getFileInfo, to populate the Info field. This probably isn't necessary, although I'm not sure why object.ensure went from getFileInfo to downloadFileByName in the first place in 8b44fc9.

kurin avatar Feb 16 '18 22:02 kurin

Oh I see, getFileInfo can't be called without the fileId, which we don't have without another call first.

However, downloadFileByName returns the X-Bz-Info-* headers, so we can populate object info from that.

kurin avatar Feb 16 '18 22:02 kurin

This is actually trickier than it looks. Not all the information (upload timestamp, status, specifically) is available from downloadFileByName. But I don't want to open with getFileInfo because identifying a 404 shouldn't be a class C op.

kurin avatar Feb 16 '18 23:02 kurin

(which is what list_files is, which is the only other way to get at the file ID when we don't already know it)

kurin avatar Feb 16 '18 23:02 kurin

You no longer need to ensure a file doesn't exist before uploading it (and that's impossible to do without race conditions anyway). Is this ticket still valid?

nicolas17 avatar Feb 28 '18 02:02 nicolas17

That's an implementation detail for restic; B2 never required that test.

This bug isn't so much about removing that test as it is determining that I'm not making two calls when one would suffice within blazer. For example, the Reader used to require a b2_get_file_info call to get the total object size before downloading it, but that was changed in #12. There may be others.

kurin avatar Feb 28 '18 03:02 kurin

Bah, I got confused and thought this ticket was in the restic repo 🙈

nicolas17 avatar Feb 28 '18 03:02 nicolas17

Hah, no worries.

kurin avatar Feb 28 '18 03:02 kurin