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

Fix empty flushed=True request for forced flushes #1628

Merged
merged 3 commits into from
Sep 7, 2022
Merged

Conversation

basepi
Copy link
Contributor

@basepi basepi commented Sep 7, 2022

What does this pull request do?

It turns out I never actually tested the functionality for sending an empty request with flushed=True when an explicit flush is called with no data on the buffer. The function signature was wrong (😅), and we also can't POST with a None body, we need an empty dict for the body.

Luckily, it should be rare to hit this issue because we usually flush before we've had a chance to send the contents of the buffer, and that flushed=True worked fine.

This fixes those issues and adds a test for this explicit case.

Thanks to @josef-cech-acc for catching this.

Related issues

Closes #1627

@apmmachine
Copy link
Contributor

apmmachine commented Sep 7, 2022

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-09-07T18:40:18.845+0000

  • Duration: 31 min 57 sec

Test stats 🧪

Test Results
Failed 0
Passed 4979
Skipped 3338
Total 8317

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /test linters : Run the Python linters only.

  • /test full : Run the full matrix of tests.

  • /test benchmark : Run the APM Agent Python benchmarks tests.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@apmmachine
Copy link
Contributor

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (66/66) 💚
Files 100.0% (225/225) 💚
Classes 100.0% (225/225) 💚
Lines 89.815% (17461/19441) 👍 0.004
Conditionals 77.133% (3191/4137) 👍 0.006

elasticapm/transport/base.py Show resolved Hide resolved
elasticapm/transport/base.py Outdated Show resolved Hide resolved
@basepi basepi requested a review from trentm September 7, 2022 18:40
@basepi basepi merged commit ccb99e5 into elastic:main Sep 7, 2022
@basepi basepi mentioned this pull request Sep 13, 2022
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.

unexpected keyword argument 'flushed' in lambda extension
3 participants