Re: [PATCH datacenter-manager 10/15] task cache: introduce ArchiveFileWriter

["Lukas Wagner" <l.wagner@proxmox.com>]

On Fri Jul 10, 2026 at 1:36 PM CEST, Dominik Csapak wrote:
>> +
>> +    /// Finalize and drop the internal writer and replace the original
>> +    /// archive file with the new contents. This method consumes `self`.
>> +    ///
>> +    /// [`ArchiveFileWriter::drop`] also finalizes the writer properly, but does not
>> +    /// allow to catch errors.
>> +    fn finalize_and_replace(mut self) -> Result<(), Error> {
>> +        self.finalize_impl()
>> +    }
>> +}
>> +
>> +impl<'a> Drop for ArchiveFileWriter<'a> {
>> +    fn drop(&mut self) {
>> +        let _ = self.finalize_impl();
>> +    }
>> +}
>> +
> isn't this a bit dangerous?
>
> this will be called whenever the writer is dropped regardless of error
> conditions and will rotate/overwrite the files.
>
> e.g. if any write in between fails, the drop handler will be called and 
> overwrite an existing archive with the partially written one?
>
> or am i missing something here?
>

You are totally right, this is indeed a bad idea, thanks for bringing it
up.

I'm using the `finalize_and_replace` method everywhere anyways, so I'd
change this to actually discard pending changes in drop, instead of
committing them. So it would just attempt to remove the temporary file
in `drop` instead of replacing the original.

Thanks!