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

[Filebeat] Fix offset field pointing at end of line for #6514 #7336

Merged
merged 1 commit into from
Jun 15, 2018

Conversation

original-brownbear
Copy link
Member

Fixes #6514. Didn't add a new test because I had to adjust that one test to reflect the change by adding EOL + message length before comparing to the stored file offset.

@ph ph self-requested a review June 14, 2018 19:49
Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

@original-brownbear can you add a changelog entry with this change?

@ph ph added review Filebeat Filebeat labels Jun 14, 2018
@original-brownbear
Copy link
Member Author

@ph sure sec :)

@original-brownbear
Copy link
Member Author

done :)

Copy link
Contributor

@ph ph left a comment

Choose a reason for hiding this comment

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

LGTM thanks @original-brownbear

@ph ph added the needs_backport PR is waiting to be backported to other branches. label Jun 14, 2018
@original-brownbear
Copy link
Member Author

@ph np + thanks for the review:
Also, sorry still new around here :) => just squash + merge and backport to what branch?

@ph
Copy link
Contributor

ph commented Jun 15, 2018

There is a test failing, I haven't see this one before, Can you take a look?

Also don't worry for the backport, I will take care of it I have a few to do.

Traceback (most recent call last):
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/case.py", line 331, in run
    testMethod()
  File "/private/var/lib/jenkins/workspace/elastic+beats+pull-request+multijob-darwin/beat/filebeat/label/macosx/src/github.com/elastic/beats/filebeat/tests/system/test_registrar.py", line 816, in test_clean_inactive
    assert len(data) == 2
AssertionError

@ph
Copy link
Contributor

ph commented Jun 15, 2018

Something tell me it might be a timing issue, I will ask jenkins to run it again.

@ph
Copy link
Contributor

ph commented Jun 15, 2018

jenkins test this please.

@original-brownbear
Copy link
Member Author

@ph likely, last night it was green and I only added the changelog entry + rebase since the last time it ran.

@ruflin
Copy link
Contributor

ruflin commented Jun 15, 2018

@ph I think it's a timing issue and i have seen the test failing before (which concerns me a bit).

@ph
Copy link
Contributor

ph commented Jun 15, 2018

@armin Yeah don't worry about it, I will followup :)
@ruflin I haven't seen it, but we did a few changes that might impact timing.

@ph ph merged commit df724c9 into elastic:master Jun 15, 2018
@original-brownbear original-brownbear deleted the 6514 branch June 15, 2018 15:05
@ph ph added v6.3.1 and removed needs_backport PR is waiting to be backported to other branches. labels Jun 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants