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

Maestro breaks if downloading twice and unexpected behaviour download function #326

Closed
magdalenafuentes opened this issue Nov 6, 2020 · 4 comments · Fixed by #414
Closed
Assignees
Labels
bug Something isn't working
Milestone

Comments

@magdalenafuentes
Copy link
Collaborator

I'm getting this error if I download maestro twice. I don't get this with other datasets (I tried with the rwc_collection and beatport_key). Even though they don't break, the partial download does different things: in the case of rwc it doesn't download a second time, in the case of beatport it downloads again.

>>> dataset.download(partial_download=['metadata'])
Starting to download ['metadata'] to folder /Users/mf3734/
> downloading metadata
440kB [00:00, 473kB/s]                                                                             
>>> dataset.download(partial_download=['metadata'])
Starting to download ['metadata'] to folder /Users/mf3734/
> downloading metadata
440kB [00:01, 247kB/s]                                                                             
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/mf3734/Documents/repos/mirdata/mirdata/core.py", line 169, in download
    cleanup=cleanup,
  File "/Users/mf3734/Documents/repos/mirdata/mirdata/datasets/maestro.py", line 273, in _download
    shutil.move(fpath, save_dir)
  File "/Users/mf3734/anaconda3/envs/mirdata/lib/python3.7/shutil.py", line 564, in move
    raise Error("Destination path '%s' already exists" % real_dst)
shutil.Error: Destination path '/Users/mf3734/maestro-v2.0.0.json' already exists
@magdalenafuentes magdalenafuentes added the bug Something isn't working label Nov 6, 2020
@rabitt rabitt self-assigned this Nov 9, 2020
@PRamoneda
Copy link
Collaborator

PRamoneda commented Nov 23, 2020

I have a similar bug.... I think...

When it is downloading a remote zip if I cancel it (^C) and I come back to download the remote a sha related error shows up :S

@PRamoneda
Copy link
Collaborator

I think that the best way for solving my bug is to add
here


this code:

except KeyboardInterrupt:
    os.remove(download_path)
    sys.exit()

@magdalenafuentes
Copy link
Collaborator Author

@PRamoneda did you check if this was solved by #344? If so we should close this issue

@magdalenafuentes magdalenafuentes added this to the 0.3 milestone Dec 15, 2020
@rabitt
Copy link
Collaborator

rabitt commented Jan 11, 2021

Looking into this:

@PRamoneda

When it is downloading a remote zip if I cancel it (^C) and I come back to download the remote a sha related error shows up :S

I think this is actually the expected behavior - the file exists but has the wrong hash, so it throws an error. This is when you can run download with force_overwrite=True, so it deletes the file. In any case, it's not clear so I'm adding some logging information for this case!

@magdalenafuentes

I'm getting this error if I download maestro twice.

There's a bug here for sure, and it should be better handled with force_overwrite - I'll add test cases for maestro and orchset and try to fix it.

While looking into it I also found weirdness in how we use cleanup, and the logic of force overwriting. Going to try to clean this up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants