copyfile: remove callback

[skshetry]

Let's remove the callback in all conditions.

The callbacks are part of fsspec, and they don't support callbacks in localfs either.
I plan on to remove tqdm and TqdmCallback from dvc-objects, which this was blocking.

Let's fix this properly if someone complains. :-)

[codecov-commenter]

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (b3a4b48) 63.71% compared to head (715ef2f) 64.16%.

Files	Patch %	Lines
src/dvc_objects/fs/local.py	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #255      +/-   ##
==========================================
+ Coverage   63.71%   64.16%   +0.44%     
==========================================
  Files          27       27              
  Lines        2067     2048      -19     
  Branches      325      319       -6     
==========================================
- Hits         1317     1314       -3     
+ Misses        690      675      -15     
+ Partials       60       59       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[efiop]

Completely removing feels like a bit too much. We should probably have a bytes-based (+number of objects) progress bar for all files, but that will still need some logic.

Let's fix this properly if someone complains. :-)

I don't think we can do it this way in this case, this is pretty important. We need some progress indication.

[efiop]

The callbacks are part of fsspec, and they don't support callbacks in localfs either.

That's not really a feature, it is just missing and needs to be added one way or another.

[skshetry]

That's not really a feature, it is just missing and needs to be added one way or another.

Either you need to support callback or not. Unfortunately there is no middleground.

[efiop]

Either you need to support callback or not. Unfortunately there is no middleground.

I'm not sure I understand why it is an unresolvable problem though.

[skshetry]

Looking through the responsibility of an API, if someone passes a callback and you decide to not create a callback, you are not honouring your part of promise here.

(Maybe there's a way to skip if we pass NoopCallback())

[efiop]

Looking through the responsibility of an API, if someone passes a callback and you decide to not create a callback, you are not honouring your part of promise here.

(Maybe there's a way to skip if we pass NoopCallback())

I agree and it probably just shouldn't create a callback on its own at all, but rather use the one provided by the user from a layer above.

[skshetry]

I agree and it probably just shouldn't create a callback on its own at all, but rather use the one provided by the user from a layer above.

But you'd still be calling callback.{set_size,relative_update,absolute_update} though.

[efiop]

@skshetry Not if you pass a None/Noop as a callback (in case of Noop I mean somehow in a way that will avoid the overhead). The most expensive part IIRC was set_size, which we can give the honor to set back to user when he creates a callback, which he can decide not to if the file is small enough or for some other reason.

[skshetry]

Can be closed, went with #256.