Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Sink::flush should return a Result, not (). #355

Open
blt opened this issue Dec 5, 2017 · 0 comments
Open

Sink::flush should return a Result, not (). #355

blt opened this issue Dec 5, 2017 · 0 comments
Labels
difficulty: moderate enhancement feature request subsystem: sinks
Milestone
0.9.0 - challenger

Comments

@blt
Copy link
Collaborator

blt commented Dec 5, 2017

Right now the flush function has no way of signaling error. As @pulltab noted the InfluxDB sink flush will loop infinitely without an influxdb to report to, which is... not great. #353 is caused by an inability to signal error upward.

This can be repaired by having Result kicked out of the function, or some other kind of failure.
@blt blt added this to the 0.8.0 - snoopy milestone Dec 5, 2017
blt pushed a commit that referenced this issue Dec 5, 2017
Avoid catastrophic failure when an ES request times out
32eab81 
As of 0.20.6 ES it's possible for a bulk request to time out,
which we unwrapped on like goofs. This commit removes that
unwrap and fails the flush in the event of a timeout bubbling up.

This resolves #353 and elastic-rs/elastic#286.

Related to #355.

Signed-off-by: Brian L. Troutwine <blt@postmates.com>
blt pushed a commit that referenced this issue Dec 5, 2017
Avoid catastrophic failure when an ES request times out
ad4d8f7 
As of 0.20.6 ES it's possible for a bulk request to time out,
which we unwrapped on like goofs. This commit removes that
unwrap and fails the flush in the event of a timeout bubbling up.

This resolves #353 and elastic-rs/elastic#286.

Related to #355.

Signed-off-by: Brian L. Troutwine <blt@postmates.com>
@blt blt mentioned this issue Dec 5, 2017
Merged
blt added a commit that referenced this issue Dec 5, 2017
@blt
Avoid catastrophic failure when an ES request times out (#356)
859ca8a 
As of 0.20.6 ES it's possible for a bulk request to time out,
which we unwrapped on like goofs. This commit removes that
unwrap and fails the flush in the event of a timeout bubbling up.

This resolves #353 and elastic-rs/elastic#286.

Related to #355.

Signed-off-by: Brian L. Troutwine <blt@postmates.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
difficulty: moderate enhancement feature request subsystem: sinks
Projects
None yet
Milestone
0.9.0 - challenger
Development

No branches or pull requests
1 participant
@blt