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.

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

BatchProcessor exception handling so that scheduler is not cancelled after connection error #144

Merged
merged 2 commits into from
Feb 29, 2016

Conversation

mmatloka
Copy link
Contributor

@mmatloka mmatloka commented Jan 7, 2016

See #143

Verified

This commit was signed with the committer’s verified signature.
renovate-bot Mend Renovate
…after connection error
@mmatloka
Copy link
Contributor Author

@majst01 ??

@mmatloka
Copy link
Contributor Author

@beckettsean ?

@beckettsean
Copy link
Contributor

@mmatloka, let's wait a bit more for @majst01 to respond. InfluxData does not maintain this repo.

@majst01
Copy link
Collaborator

majst01 commented Feb 17, 2016

Hi,
still alive, sorry for the delay, im quite bussy.

Any chance to add a unit test to verify that your code works, other than that seems correct and obviously needed.

@andrewdodd
Copy link
Contributor

This is essentially the same change that was supplied in #119 but with data loss instead of potential OOM.

I think there are some key 'feature' questions that need answers before you can really decide to include this. I have made a (unfortunately long) issue discussing this at #148.

@mmatloka
Copy link
Contributor Author

@majst01 sure, I'll add the tests!

@mmatloka
Copy link
Contributor Author

Hey @majst01,
I have added the test case.

@andrewdodd
Copy link
Contributor

I've changed my mind. I think including this is reasonable (though I'll admit I'm wary of pokemon exception handling). I think it is a simple enough change to include for now.

majst01 added a commit that referenced this pull request Feb 29, 2016
BatchProcessor exception handling so that scheduler is not cancelled after connection error
@majst01 majst01 merged commit 9ca5296 into influxdata:master Feb 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants