S3 Bids Fetch Fixes

[36000]

It seems to be working now. We made 3 fixes:

1. make directory for modality agnostic files if it does not exist
2. allow derivatives to download, which was broken before because the prefix was wrong
3. download the dataset description for derivatives by default whenever downloading a given derivative, if it exists

[richford]

I thought that this line in _download_from_s3 would take care of that. Were you experiencing an issue where it crashed if the directory didn't exist? In either case, it's probably better to solve this issue inside of _download_from_s3 so that we don't have to duplicate code in both of the download methods.

[36000]

Weird. Yah, it looks like that should cover it. Here is an example of the bug I am trying to fix: https://us-west-2.console.aws.amazon.com/cloudwatch/home?region=us-west-2#logsV2:log-groups/log-group/$252Faws$252Fbatch$252Fjob/log-events/afq_cam_can-64gb-20810-0-cloudknot-job-definition$252Fdefault$252Ffcee42ed-3588-46c8-907f-a6904d7743ad

[richford]

Ahh, I see. It's running into the same issue for the modality agnostic files. I think the fix in #341 is probably a better way to go. Feel free to take the changes from that one and close that PR if you want to keep this one open.

[36000]

I think I will keep this open and throw stuff in here, then I can drop commits that you make better fixes for elsewhere. This PR is really useful, so I am eager to get this working with cam-can so I can use it with other datasets as well.

[36000]

So it looks like _ls_s3fs returns the full prefix to a file, but sometimes we were using it like it returned only the last part of the prefix. Anyways, with these fixes, I can download derivatives.

[pep8speaks]

Hello @36000! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-08-12 15:37:23 UTC

[arokem]

This looks good to me, but I am not 100% sure that I was following the discussion. @richford : would you mind taking a look and giving this a 👍 / 👎 ?

[richford]

@36000, could you remove line 721 from data.py? This is a bug that I just found and I figure we should just fix it in this PR instead of opening a new one. Other than that, LGTM!