Daijiro Fukuda

Results 519 comments of Daijiro Fukuda

Thanks!! I will see this! > I guess the CI test fail is not related to this change Yes, They are not related.

> Now since we have implemented stream directly so the raise not needed Yes, we don't need `raise` in the new implementation. However, `router.emit_error_event` should be called as before.

> https://github.com/fluent/fluentd/blob/51b860b1be2eb59076d706fda12d55657a614c8f/test/plugin/test_filter_parser.rb#L659-L666 > > could you please point which emit_event is this call part of In the current implementation: https://github.com/fluent/fluentd/blob/78a7972bfe6b421f08472701f04f00515ed24bee/lib/fluent/plugin/filter_parser.rb#L114-L119 In the new implementation: https://github.com/fluent/fluentd/blob/9ec5a95c964cb2eb6340b7538e25c8547d4bbaec/lib/fluent/plugin/filter_parser.rb#L105-L108 This change is OK because...

> @daipom i am not sure what is this issue > > ``` > Failure: test_filter_key_not_exist(ParserFilterTest): > in mock 'flexmock(Fluent::Test::Driver::TestEventRouter)': no matching handler found for emit_error_event("test", 1272942121, {"foo"=>"bar"}, #) >...

I made another PR to add some test cases for abnormal cases. * #4638 Let's rebase after it is merged. I confirmed the test results on my local for this...

@ashie @kenhys Which version should we release this on? This solves the limitation of #4474, which is released on v1.17.0. It's ambiguous whether it's a bug fix or an enhancement....

> Thanks! > I have confirmed this change passes tests on #4638. > I believe we are now ready to merge! (after #4638) Thanks so much for this fix! Now...

Sorry I haven’t been able to make time for this PR. > I'd be happy to continue to work on it to get this merged. Is the [DCO](https://github.com/fluent/fluent-plugin-kafka/pull/521/checks?check_run_id=34104572736) the reason...

Sorry for my late response. Thanks! It looks like the issue was that the Git history had become complicated. So, it looks better to either rebase and force push or...