Consider moving Downloader to a Brackground processer

[Davidslv]

Hi,

I've noticed that the PullRequestDownloadsController uses Downloader service in a synchronous way. I was wondering if moving it to a background process (like using http://sidekiq.org/) would make sense to you?

I just wanted to be sure if that was intentional not to have Sidekiq (or other) or it still something in your TODO list of things

Thanks.

[adamyeats]

Hey @Davidslv! I think it could be moved to a background service, absolutely. However, there is potential complexity on line 5, as Downloader.new(current_user).get_pull_requests seems to want to be called before the pull_request_downloads/create.html.erb partial is rendered.

Based on a 3 minute glance at the code, the partial is not dependent on anything returned by this method: it's my assumption that it updates our DB in the background before passing execution on to the library that queries the Github API, so I can forsee some issues where the state we have in our DB is stale when the template is rendered on the first time, but maybe not on subsequent renders.

So TL;DR: there's some refactoring involved here. My question to you would be: if you used a Sidekiq worker here, how would you best ensure the data we had at render time would be fresh?

[andrew]

Most of the use of the Downloader class is in rake tasks that are run in the background

I've not moved it to a background queue as on heroku it adds extra actual monetary costs that would require extra sponsorship 🤑 It would also require something to push back to the web page to say when the background task is finished syncing manually from the dashboard