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

[Packetbeat] HTTP: Improve support for 100-continue #15830 #19349

Merged
merged 7 commits into from
Jul 27, 2020
Merged

[Packetbeat] HTTP: Improve support for 100-continue #15830 #19349

merged 7 commits into from
Jul 27, 2020

Conversation

OhBonsai
Copy link
Contributor

@OhBonsai OhBonsai commented Jun 24, 2020 •
edited
Loading

What does this PR do?

Improve support for 100-continue #15830

Checklist

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works

How to test this PR locally

nosetests ./tests/system/test_0070_http_100_continue.py

Related issues

#15830

@OhBonsai OhBonsai requested a review from a team as a code owner June 24, 2020 05:07
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

1 similar comment
@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Jun 24, 2020
@cla-checker-service
Copy link

cla-checker-service bot commented Jun 24, 2020 •
edited
Loading

💚 CLA has been signed

@elasticmachine
Copy link
Collaborator

elasticmachine commented Jun 24, 2020 •
edited by jenkins-beats-ci bot
Loading

💚 Build Succeeded

Pipeline View Test View Changes Artifacts preview

Expand to view the summary

Build stats

  • Build Cause: [marc-gr commented: jenkins run tests]

  • Start Time: 2020-07-27T07:54:23.361+0000

  • Duration: 44 min 6 sec

Test stats 🧪

Test Results
Failed 0
Passed 1061
Skipped 12
Total 1073

@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Jun 24, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/siem (Team:SIEM)

@marc-gr
Copy link
Contributor

marc-gr commented Jul 15, 2020

Thanks for your first PR @OhBonsai !

Please we need you to sign our CLA, you can follow instruction for this here.

@marc-gr
Copy link
Contributor

marc-gr commented Jul 15, 2020

jenkins run tests

@OhBonsai
Copy link
Contributor Author

@marc-gr
I have signed many times.. But it's seems not working... Anything wrong?
image

@marc-gr
Copy link
Contributor

marc-gr commented Jul 16, 2020

@marc-gr
I have signed many times.. But it's seems not working... Anything wrong?
image

It seems to be OK now, maybe the email used to commit was different than the one you used to sign before. Thanks!

@marc-gr
Copy link
Contributor

marc-gr commented Jul 16, 2020

jenkins run tests

@marc-gr
Copy link
Contributor

marc-gr commented Jul 17, 2020

jenkins run tests

@marc-gr
Copy link
Contributor

marc-gr commented Jul 17, 2020

It seems it has linting issues:

+++ b/packetbeat/tests/system/test_0070_http_100_continue.py
@@ -30,5 +30,3 @@ class Test(BaseTest):
         assert "response" in o["http"]
 
         assert not "error" in o
-
-
Error: some files are not up-to-date. Run 'mage fmt update' then review and commit the changes. Modified: [packetbeat/tests/system/test_0070_http_100_continue.py]

You can fix this by running mage fmt update in packetbeat folder and commiting its output. Also an entry in the CHANGELOG.next.asciidoc needs to be added. Let us know if you have any issues or want us to take over from this point, whatever works best for you :)

Thanks!

@OhBonsai
Copy link
Contributor Author

@marc-gr To Be Reviewed

@marc-gr
Copy link
Contributor

marc-gr commented Jul 17, 2020

jenkins run tests

@marc-gr
Copy link
Contributor

marc-gr commented Jul 21, 2020

jenkins run tests

@marc-gr
Copy link
Contributor

marc-gr commented Jul 21, 2020

@OhBonsai I merged master since tests were red and your branch was quite behind

@OhBonsai
Copy link
Contributor Author

@marc-gr Thanks... So there is anything else I need to do ?

@marc-gr marc-gr requested a review from adriansr July 22, 2020 14:15
@marc-gr
Copy link
Contributor

marc-gr commented Jul 22, 2020

@marc-gr Thanks... So there is anything else I need to do ?

I would like @adriansr to have a look since he is more familiar with packetbeat 👍

@adriansr adriansr self-assigned this Jul 22, 2020
adriansr
adriansr suggested changes Jul 22, 2020
View reviewed changes
packetbeat/protos/http/http.go Outdated Show resolved Hide resolved
@OhBonsai @adriansr
Update packetbeat/protos/http/http.go
7252e4c 
Co-authored-by: Adrian Serrano <adrisr83@gmail.com>
@adriansr
Copy link
Contributor

jenkins, test this

@marc-gr
Copy link
Contributor

marc-gr commented Jul 27, 2020

jenkins run tests

@marc-gr marc-gr merged commit 41bc8c6 into elastic:master Jul 27, 2020
marc-gr pushed a commit to marc-gr/beats that referenced this pull request Jul 27, 2020
@OhBonsai @marc-gr
[Packetbeat] HTTP: Improve support for 100-continue elastic#15830 (el…
ce39f95 
…astic#19349)

* refactor(packet beat): Improve support for 100-continue

* test(packetbeat): 100-continue only generate one event without error

* test(packetbeat): 100-continue only generate one event without error

* Update packetbeat/protos/http/http.go

Co-authored-by: Adrian Serrano <adrisr83@gmail.com>

* delete unused string

* Fix format issue

Co-authored-by: Marc Guasch <marc.guasch@elastic.co>
Co-authored-by: Adrian Serrano <adrisr83@gmail.com>
(cherry picked from commit 41bc8c6)
@marc-gr marc-gr mentioned this pull request Jul 27, 2020
Merged
3 tasks
v1v added a commit to v1v/beats that referenced this pull request Jul 27, 2020
@v1v
Merge remote-tracking branch 'upstream/master' into feature/ci-pipeli…
8175b5a 
…ne-2.0

* upstream/master: (41 commits)
  adding possibility to override content-type checks, it was breaking certain webhooks that is not able to set content-headers at all. Still defaults to application/json (elastic#20232)
  fix: use a fixed worker type for tests (elastic#20130)
  [Ingest Manager] Prepare packaging for endpoint and asc files (elastic#20186)
  [Packetbeat] HTTP: Improve support for 100-continue elastic#15830 (elastic#19349)
  Increase index.max_docvalue_fields_search to 200 (elastic#20218)
  [Ingest Manager] Prevent closing closed reader (elastic#20214)
  [Metricbeat] Use MySQL Host Parser in Query metricset (elastic#20191)
  Testing: Ignore timestamp from cylance/protect dataset (elastic#20211)
  [Filebeat] Ignore cylance.protect timestamps while testing (elastic#20207)
  [CI] remove codecov step (elastic#20102)
  [docs] Indicate that SYSTEM user is required on Windows to use Endpoint (elastic#20172)
  Remove f5/firepass rsa2elk fileset (elastic#20160)
  [Elastic Agent] Improve GRPC stop to be more relaxed. (elastic#20118)
  Fix fileset field prefixing (elastic#20170)
  Fix terminating pod autodiscover issue (elastic#20084)
  Call host parser only once when building light metricsets (elastic#20149)
  [CI] fix null string with contains (elastic#20182)
  [Ingest Manager] Fix failing unit tests on windows (elastic#20127)
  [Filebeat] Update crowdstrike module (elastic#20138)
  [docs] Add x-pack role to relevant metricsets (elastic#20167)
  ...
marc-gr added a commit that referenced this pull request Jul 28, 2020
@marc-gr @OhBonsai
[Packetbeat] HTTP: Improve support for 100-continue #15830 (#19349) (#…
a6a9cd5 
…20234)

* refactor(packet beat): Improve support for 100-continue

* test(packetbeat): 100-continue only generate one event without error

* test(packetbeat): 100-continue only generate one event without error

* Update packetbeat/protos/http/http.go

Co-authored-by: Adrian Serrano <adrisr83@gmail.com>

* delete unused string

* Fix format issue

Co-authored-by: Marc Guasch <marc.guasch@elastic.co>
Co-authored-by: Adrian Serrano <adrisr83@gmail.com>
(cherry picked from commit 41bc8c6)

Co-authored-by: Bonsai <LetBonsaiBe@gmail.com>
melchiormoulin pushed a commit to melchiormoulin/beats that referenced this pull request Oct 14, 2020
@OhBonsai @marc-gr
@adriansr @melchiormoulin
[Packetbeat] HTTP: Improve support for 100-continue elastic#15830 (el…
0b280d2 
…astic#19349)

* refactor(packet beat): Improve support for 100-continue

* test(packetbeat): 100-continue only generate one event without error

* test(packetbeat): 100-continue only generate one event without error

* Update packetbeat/protos/http/http.go

Co-authored-by: Adrian Serrano <adrisr83@gmail.com>

* delete unused string

* Fix format issue

Co-authored-by: Marc Guasch <marc.guasch@elastic.co>
Co-authored-by: Adrian Serrano <adrisr83@gmail.com>
@marc-gr marc-gr mentioned this pull request Oct 22, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adriansr adriansr adriansr approved these changes

Assignees

@adriansr adriansr

Labels
enhancement Packetbeat v7.10.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@OhBonsai @elasticmachine @marc-gr @adriansr @andresrc