rabbit icon indicating copy to clipboard operation
rabbit copied to clipboard

Consumer sync start

Open Odaeus opened this issue 3 years ago • 3 comments

Resolves #68.

This adds sync_start as an option to the Consumer init. Hopefully causing the init process not to return control until the consume is ready to consume, for processes that wish to wait on this.

I wasn't quite sure the best way to test this, let me know if you have thoughts on how it could be improved.

Odaeus avatar Mar 15 '21 16:03 Odaeus

@Odaeus This looks great!

In terms of testing - I'm wondering if we should add an extra attribute to the internal server state - something akin to :sync_start_run that defaults to false. Once sync_start/2 completes - this is updated to true. We can then assert that it ran based on this value from the state. Thoughts?

nsweeting avatar Mar 16 '21 19:03 nsweeting

@nsweeting I took that idea with a slight twist, the value :started_mode defaults to :async and updates to :sync only if the sync_start function completes. You can detect two things with this: whether the sync_start option is correctly used, and if sync_start actually failed and it reverted to asynchronous start. Obviously the latter is would also be equivalent to %{sync_start_run: false, consuming: true}, I just felt it was clearer with an atom.

I'm not particularly attached to it, so can use your original suggestion if you prefer.

Odaeus avatar Mar 17 '21 15:03 Odaeus

@nsweeting just wanted to check-in and ask if you have any time to review this?

Odaeus avatar Apr 29 '21 11:04 Odaeus