Skip to content

Conversation

clementzach
Copy link

@clementzach clementzach commented Oct 17, 2022

This PR adds a helper function that allows a user to easily download data from a whole study.

One question: Is there a reason that BACKFILL_WINDOW is set to be 5? It seems like that slows down the process of downloading data a good amount.

In my experience, we should be able to have a default value of BACKFILL_WINDOW that is at least 100 or so.

@biblicabeebli
Copy link
Member

@tokeetim would you like me to do some review here to speed the process along?

@tokeefe
Copy link
Contributor

tokeefe commented Oct 18, 2022

@biblicabeebli Yes, please! Thank you!

@biblicabeebli
Copy link
Member

Crap I just finished the modernizing/cleanup/python-3.8-fixes refactor I was working on and then looked at this. I totally forgot this pull existed.

@clementzach the refactor for the sync file was just to move it to a file called sync (should maintain import compatibility) from the /sync/__init__.py. I could probably pull in your code additions in manually but that then it wouldn't be attributed to you. Would you like me to do that (because it's been..... a while....) or would you like to handle the merge/rebase?

I'm still new to this codebase, I will start using it, I don't know why a longer window would speed up/slow down requests. One change I did make internally that may affect speed was to up an internal chunking value. Otherwise, I intend to try and get into this code a little more for speed and, eventually, a v2 download endpoint that pulls actually-compressed data.

(it was once compressed! ... but there was a server-destroying memory not-exactly-a-leak-but-kinda-a-leak and I had to turn it off...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants