Consumer sync start

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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

[Odaeus]

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.

[nsweeting]

@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?

[Odaeus]

@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]

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