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

Limit simultaneous downloads by ip address #39

Merged
merged 14 commits into from
Jan 6, 2020
Merged

Conversation

sfisher
Copy link
Contributor

@sfisher sfisher commented Jan 4, 2020

See datadryad/dryad-product-roadmap#613 .

I've added testing where I can but this is a bit difficult to test because of concurrency.

This is a good dataset for testing against since it has a large file/dataset that takes a while to download https://dryad-dev.cdlib.org/stash/dataset/doi:10.7959/dryad.8cz8w9j1 .

Essentially this logs downloads to the stash_engine_download_histories table. Create a record when a download starts. state is downloading or finished. Change to finished when completed. The difference between created_at and updated_at times is the download length. Write ip_address and user_agent to table and I hope this will make it easier for us to see the download activity and see what is going on instead of searching through logs.

Also added rake task and the other config repo adds it to daily cron to cleanup older downloads than 60 days and fix suspicious states that never go to finished (which mostly shouldn't happen unless there is an ungraceful shutdown of a crashed server or something).

Added tests .

@sfisher sfisher requested a review from ryscher January 4, 2020 01:35
@ryscher
Copy link
Member

ryscher commented Jan 6, 2020

Thanks, Scott! This looks great, and works well in my testing.

I corrected one minor rubocop issue.

@ryscher
Copy link
Member

ryscher commented Jan 6, 2020

Not merging yet, since there is still a testing issue with counter_stat_spec.

@sfisher
Copy link
Contributor Author

sfisher commented Jan 6, 2020

Woot! Got tests to pass. It was some weird thing where it was calculating the year and week strangely (2020-week 52) instead of (2019-52).

@ryscher ryscher merged commit 3432042 into master Jan 6, 2020
@sfisher sfisher deleted the download-limits branch June 11, 2021 19:54
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.

2 participants