WIP - Event

[williampj]

events_request_spec is still outstanding

[AndiLavera]

This is a really creative implementation but i don't think circumventing sidekiqs retries is the best solution. Sidekiq encourages the use of it's middleware api. Here is a stackoverflow post about dynamic retries which is what we are trying to accomplish:

https://stackoverflow.com/questions/23065578/sidekiq-retry-count-in-job

Furthermore, I don't see how you would test your custom retry implementation. Like how would you throw an error once or twice? If we use sidekiqs retry api, we don't need to test it.

[williampj]

Sidekiq automatically does 25 retries over 21 days by default if you don't set retry to false or 0. We need to deactivate that if we're to let the user control the delay and retries. I tried working with setting the retry option dynamically based on bridge.retries, but I ran into issues with that so I went with doing it manually and maintaining complete control. It's a week ago so don't remember what they were exactly, but it was a pain. Right now it's dynamic in that the user sets retries and it's pulled from bridge.retries. I'll look at your SO link.

I'm writing the events_request_spec today that will test it. If you send an http request that throws an http error, then that error will be thrown for each iteration and stored in the data object. So the error contents of the data object can be tested. Remember the outbound key stores an array of request-response cycles. The implementation is in the save_http_error and create_error_response methods.

[AndiLavera]

That all sounds fine. I have 1 other major concern.

We can theoretically only run 5 jobs at one time. How are you going to do the delay function? If 5 jobs are running, and you use "sleep" or any other waiting method, those 5 jobs are stilling going. Those 5 jobs in sleep mode or whatever will hold the entire queue. If those delays are 1hr, we would have to wait 1hr until the queue can start moving again.

This is where sidekiqs retry & delay really shine. A delay between retries won't hold up the queue but a delay during your custom retries will. How do you plan to remedy that while circumventing sidekiqs retry api?

[williampj]

Merde...
Back to the drawing board

[AndiLavera]

Why date & time? Why not dateTime? I can format it on the frontend. JS is much faster than ruby too!

[williampj]

That was actually for frontend user friendliness, so that the four pieces of info you need each have a key. But no problem if you want to handle in on that end.

[AndiLavera]

This would be really slow. Instead of iterating, why not push this kind of this into the db?

Event.find(event_params[:event_id]).bridge.events.limit(100).order(completed_at: :desc)

That would make a db query similar to:

SELECT * FROM events
WHERE bridge_id = x
LIMIT 100
ORDER_BY completed_at DESC

[williampj]

The main point of that method is to format each event object to the format of the sidebar, which requires iteration.
If I add your limit(100).order(completed_at: :desc) to the events being passed in, that will speed it up some by alleviating the sort_by and reverse calls here. I suppose that's also what you mean

[AndiLavera]

Yeah lets format it on the frontend. You can just send me the raw timestamp. I already have to iterate through events. No reason to add another iteration before.

[williampj]

I can see I've wasted a lot of time massaging data for easy front-end consumption. Next time I shouldn't look at the design or UI when writing controllers, but just a list of data needs for each action/endpoint.

[AndiLavera]

These should be using the includes query method.

https://apidock.com/rails/ActiveRecord/QueryMethods/includes

[williampj]

Ok, I'll work on a more efficient way of loading these.

[AndiLavera]

Closed for #28