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

Fixing Faraday::RackBuilder::StackLocked #1371

Merged
merged 3 commits into from
Mar 9, 2023
Merged

Fixing Faraday::RackBuilder::StackLocked #1371

merged 3 commits into from
Mar 9, 2023

Conversation

ericproulx
Copy link
Contributor

@ericproulx ericproulx commented Mar 3, 2023 •
edited
Loading

This PR is intended to fix #1370.

What does this pull request do?

This PR removes Faraday's tracing middleware and replaces it by a begin/rescue of Faraday's ClientError and ServerError.

Why is it important?

It fixes a bug in 4.6.1. release

Checklist

  • [ x ] I have signed the Contributor License Agreement.
  • [ x ] My code follows the style guidelines of this project (See .rubocop.yml)
  • [ x ] I have rebased my changes on top of the latest main branch
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ x ] New and existing unit tests pass locally with my changes

Related issues

@apmmachine
Copy link
Contributor

apmmachine commented Mar 3, 2023 •
edited
Loading

💚 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 previewSnapshots

Expand to view the summary

Build stats

  • Start Time: 2023-03-09T08:50:38.682+0000

  • Duration: 30 min 46 sec

Test stats 🧪

Test Results
Failed 0
Passed 43609
Skipped 77
Total 43686

🤖 GitHub comments

Expand to view the GitHub comments

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

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark test.

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

@ericproulx ericproulx mentioned this pull request Mar 6, 2023
Closed
@ericproulx
Copy link
Contributor Author

/test

@estolfo
Copy link
Contributor

estolfo commented Mar 6, 2023

jenkins test please

@apmmachine
Copy link
Contributor

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 99.237% (130/131) 👍
Classes 99.237% (130/131) 👍
Lines 57.206% (2469/4316) 👍 0.106
Conditionals 100.0% (0/0) 💚

@estolfo
Copy link
Contributor

estolfo commented Mar 6, 2023

@ericproulx I ran the tests and the failures are not related to these changes. I'll need to take a closer look and possibly merge this PR in anyway so it's not sitting too long until I fix the test failures. Thanks for your patience.

@estolfo
Copy link
Contributor

estolfo commented Mar 7, 2023

Just FYI, the test failures are due to the merge of this PR, reported in this issue.

@estolfo
Copy link
Contributor

estolfo commented Mar 7, 2023

@ericproulx can you rebase? I resolved the test failures in main.

@ericproulx ericproulx force-pushed the fixing_stack_locked branch from 960793c to 939e06e Compare March 8, 2023 12:08
@ericproulx
Copy link
Contributor Author

@ericproulx can you rebase? I resolved the test failures in main.

Done

@estolfo estolfo enabled auto-merge (squash) March 9, 2023 10:45
@estolfo estolfo disabled auto-merge March 9, 2023 10:45
@estolfo estolfo merged commit a709dcb into elastic:main Mar 9, 2023
@estolfo
Copy link
Contributor

estolfo commented Mar 9, 2023

Thanks again @ericproulx !

@lancedikson
Copy link

Hi folks! We also encountered this issue and would like to know when you guys are planning to make a release including this fix? Thanks!

@ericproulx ericproulx mentioned this pull request Mar 21, 2023
Closed
jclusso added a commit to jclusso/apm-agent-ruby that referenced this pull request Apr 13, 2023
@jclusso
Merge remote-tracking branch 'elastic/main'
e81cec0 
* elastic/main: (30 commits)
  docs: remove kibana endpoint (elastic#1381)
  Update status badge (elastic#1379)
  Create single status check that can be set as required (elastic#1378)
  Remove jenkins related precommit hooks (elastic#1380)
  Migrate Jenkinsfile 2 GH Actions Workflow (elastic#1366)
  Migrate update specs to updatecli (elastic#1375)
  v4.6.2
  Fixing Faraday::RackBuilder::StackLocked (elastic#1371)
  Fix jruby docker images (elastic#1367)
  Update reference to sinatra main (elastic#1373)
  Update release:update_branch task to reference branch 4.x
  Add missing docs reference
  v4.6.1
  Add security options to docker containers (elastic#1356)
  Make sure http status code is set when Faraday Middleware is used (elastic#1368)
  Use composite action for updatecli workflow (elastic#1365)
  Fix sha source in updatecli update-specs.yml (elastic#1363)
  Add update-specs updatecli workflow (elastic#1359)
  use jruby user to run docker containers (elastic#1355)
  Close the read pipe at the right moment (elastic#1351)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@estolfo estolfo estolfo approved these changes

Assignees
No one assigned
Labels
agent-ruby
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Faraday errors after updating to 4.6.1
4 participants
@ericproulx @apmmachine @estolfo @lancedikson