Skip to content

Save last_error on init and emit - #29

Closed
cabiad wants to merge 3 commits into
fluent:masterfrom
tophat-archive:upstream
Closed

Save last_error on init and emit#29
cabiad wants to merge 3 commits into
fluent:masterfrom
tophat-archive:upstream

Conversation

@cabiad

@cabiad cabiad commented Feb 2, 2015

Copy link
Copy Markdown

Uses a threadlocal variable to store errors, which a user of this library can optionally access and act on after initializing the sender (initially connecting to the fluentd server) or emitting an event.

Also includes relevant tests.

Comment thread setup.py Outdated

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoops, I'll resubmit in a sec without this change

@cabiad

cabiad commented Feb 24, 2015

Copy link
Copy Markdown
Author

@kiyoto - I just wanted to follow up on our earlier email discussion. Do you think this PR can be merged?

@cabiad

cabiad commented Mar 11, 2015

Copy link
Copy Markdown
Author

🔔 - Any update on this?

@kiyoto

kiyoto commented Mar 11, 2015

Copy link
Copy Markdown
Contributor

Ah sorry. @yyuu and @kzk do you think you can review and merge the changes?

@yyuu

yyuu commented Mar 12, 2015

Copy link
Copy Markdown
Member

@kiyoto The test code won't success if fluentd isn't listening on 24224/tcp. It should be rewritten with using mock to pass tests on travis-ci.org.

// I found travis-ci.org is not working on fluent/fluent-logger-python. See also #32

@cabiad

cabiad commented Mar 12, 2015

Copy link
Copy Markdown
Author

@yyuu and @kiyoto - The existing tests also fail in that case, but because error information was discarded, the failure was not detectable. I added comments to indicate this where appropriate to existing tests.

I believe that this is therefore an important fix. In my opinion, it makes sense to take this increment of value right away. We can update all tests to work properly without a running fluentd instance in another PR.

@ghost

ghost commented Apr 15, 2016

Copy link
Copy Markdown

Hiding the connect or send error is crazy, this library needs to be fixed. Look how the ruby library returns a success or failure by bubbling up a true or false on the post() method here:
https://github.com/fluent/fluent-logger-ruby/blob/master/bin/fluent-post

This lib should ideally do the same, beside storing the error, a new event should have a post() method and that should return a true/false on success or not. That makes usage far more clear IMO.

repeatedly added a commit that referenced this pull request Jul 25, 2016
This patch is based on cabiad's patch.
#29
@repeatedly repeatedly mentioned this pull request Jul 25, 2016
@repeatedly

Copy link
Copy Markdown
Member

I merged #59.
This patch is baesd on this PR with additional changes.
Thanks for the contribution!

@repeatedly repeatedly closed this Jul 26, 2016
@tophat-jenkins
tophat-jenkins deleted the upstream branch June 29, 2022 20:13
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.

4 participants