pkordel / multisearch

Search for multiple keywords in one go
0 stars 0 forks source link

Lack of asynchronicity in the report creation process #15

Open olivierobert opened 4 years ago

olivierobert commented 4 years ago

While the decoupling of the report creation and search result fetching processes is a good decision, the services are called synchronously in the context of the controller ReportsController:

https://github.com/pkordel/multisearch/blob/8b2699ffaa80e223ced6904edf79d121fbe2a0e3/app/controllers/reports_controller.rb#L19-L21

Report::Create then call SearchResult::Create synchronously as well:

https://github.com/pkordel/multisearch/blob/8b2699ffaa80e223ced6904edf79d121fbe2a0e3/app/services/report/create.rb#L20-L22

Since SearchResult::Create loops through all keywords to parse the search results, this process could take a long time. So a good practice is to process this asynchronously. That's where lies the difficulty of the code challenge and where we expect applicants to make this architecture decision independently. While the implementation might vary (as there are many options to do it :-)), the asynchronous process must be there.

As a side note, using sleep is also a code smell and should be used as the last resort:

https://github.com/pkordel/multisearch/blob/8b2699ffaa80e223ced6904edf79d121fbe2a0e3/app/services/search_result/create.rb#L18-L23

olivierobert commented 4 years ago

Here is a recording of what happens when fetching for only 3 keywords which takes a long time:

2563-10-22 12 00 14

pkordel commented 4 years ago

@olivierobert yes everything you mention is stuff I intended to address. This was meant to be an initial commit to serve as a basis for collaboration. Frankly, I was unaware of the challenges posed by Google search wrt scraping. Trying various approaches ate up a lot of my time.

olivierobert commented 4 years ago

We need to treat the submission as the most complete you could do in the provided time of one week. If you shared that to us a few days before the one-week deadline, we would consider it as work-in-progress but not after one week. I hope you understand we need to assess all candidates on the same basis of time.

pkordel commented 4 years ago

Yes completely fair. I interpreted this document: https://github.com/nimblehq/our-team/blob/master/join-us/our-recruitment-process.md to mean that the allotted time for completion is one week, followed by a code review taking 1-3 days. During which time we would collaborate and issues could be fixed. My apologies if I misread the guidelines.

On Thu, 22 Oct 2020 at 13:55 Olivier Robert notifications@github.com wrote:

We need to treat the submission as the most complete you could do in the provided time of one week. If you shared that to us a few days before the one-week deadline, we would consider it as work-in-progress but not after one week. I hope you understand we need to assess all candidates on the same basis of time.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/pkordel/multisearch/issues/15#issuecomment-714274654, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABNDVTNGU6FBZZKTRH75LSL7JMRANCNFSM4S2U4GHQ .