irmin-pack: update flush, close and reload to return `Ro_not_allowed instead of raising an exception

[zshipko]

Closes #2043

[codecov-commenter]

Codecov Report

Merging #2044 (62e4641) into main (b5d5e3f) will decrease coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #2044      +/-   ##
==========================================
- Coverage   64.48%   64.43%   -0.06%     
==========================================
  Files         130      130              
  Lines       15604    15607       +3     
==========================================
- Hits        10063    10056       -7     
- Misses       5541     5551      +10     

Impacted Files	Coverage Δ
src/irmin-pack/unix/pack_index.ml	63.88% <0.00%> (-5.81%)	⬇️
src/irmin-fs/unix/irmin_fs_unix.ml	64.51% <0.00%> (-3.23%)	⬇️
src/irmin-fs/irmin_fs.ml	81.21% <0.00%> (-1.22%)	⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[metanivek]

Thanks! Could you look to see if octez code needs updating with the new error?

[metanivek]

Since v does not use Ro_not_allowed it might be nice to not include it in its error type but I'm not sure of good names to distinguish the two categories of error. My first thought was create_error and write_error but write seems a little too broad. What do you think?

[zshipko]

write_error sounds fine, or we could do create_error and keep error (which is even more generic 😄)

[metanivek]

kk, lets go with write_error

[zshipko]

Okay, I split out the write_error and create_error

Thanks! Could you look to see if octez code needs updating with the new error?

It seems unaffected

[metanivek]

Thanks for checking. To satisfy my own curiosity, I just took a look to answer the question of why not. Turns out we raise exceptions on error at the store boundary:

irmin/src/irmin-pack/unix/ext.ml

Line 258 in b5d5e3f

let flush t = File_manager.flush ?hook:None t.fm |> Errs.raise_if_error

So now callers will get a Ro_not_allowed exception instead of an assertion failure, which is a better situation.