Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create error handler for remotes #2965

Closed
pared opened this issue Dec 17, 2019 · 4 comments
Closed

Create error handler for remotes #2965

pared opened this issue Dec 17, 2019 · 4 comments
Labels
discussion requires active participation to reach a conclusion enhancement Enhances DVC p2-medium Medium priority, should be done, but less important

Comments

@pared
Copy link
Contributor

pared commented Dec 17, 2019

As in
#2866 (comment)

@MrOutis @Suor @efiop
I would like to ask your opinions on that, though AFAIK, @efiop agrees.

@triage-new-issues triage-new-issues bot added the triage Needs to be triaged label Dec 17, 2019
@pared pared added the discussion requires active participation to reach a conclusion label Dec 17, 2019
@triage-new-issues triage-new-issues bot removed the triage Needs to be triaged label Dec 17, 2019
@efiop efiop added enhancement Enhances DVC p2-medium Medium priority, should be done, but less important labels Dec 17, 2019
@efiop
Copy link
Contributor

efiop commented Dec 17, 2019

So the change would be that each _download/_upload knows which specific errors to catch (e.g. ClientError for s3) and the rest falls through and is either being caught in RemoteBASE.download/upload if it is something common but remote-related, or it is caught by main.py (e.g. ulimit issue). Currently we catch everything in RemoteBASE.download/upload and treat it as something recoverable, which is not the case.

@Suor
Copy link
Contributor

Suor commented Dec 17, 2019

I don't see much value in this refactoring unless we actually do something useful with that specific errors, i.e. retry or dynamic_chunk_size. Otherwise we will just add lots of code, which may make it harder for us to actually do something useful later.

@ghost
Copy link

ghost commented Dec 20, 2019

Can't imagine how would it look like, @pared, but I agree that we should treat exceptions better

@efiop
Copy link
Contributor

efiop commented May 3, 2021

Migrating to fsspec that adapts cloud specific errors to OSError's and even retries stuff if needed. Closing as stale.

@efiop efiop closed this as completed May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion requires active participation to reach a conclusion enhancement Enhances DVC p2-medium Medium priority, should be done, but less important
Projects
None yet
Development

No branches or pull requests

3 participants