Skip to content

Fix file descriptors (sockets) leak due to leak urlopen requests#4294

Merged
puremourning merged 2 commits intoycm-core:masterfrom
alxn1:master
Dec 30, 2025
Merged

Fix file descriptors (sockets) leak due to leak urlopen requests#4294
puremourning merged 2 commits intoycm-core:masterfrom
alxn1:master

Conversation

@alxn1
Copy link
Contributor

@alxn1 alxn1 commented Mar 10, 2025

PR Prelude

  • I have read and understood YCM's CONTRIBUTING document.
  • I have read and understood YCM's CODE_OF_CONDUCT document.
  • I have included tests for the changes in my PR. If not, I have included a
    rationale for why I haven't.
  • I understand my PR may be closed if it becomes obvious I didn't
    actually perform all of these steps.

Why this change is necessary and useful

This PR should fix problem described in #4293


This change is Reviewable

@alxn1 alxn1 closed this Mar 10, 2025
@alxn1 alxn1 reopened this Mar 10, 2025
@alxn1 alxn1 changed the title Fix file descriptors (sockets) leak due to not closed urlopen requests Fix file descriptors (sockets) leak due to leak urlopen requests Mar 10, 2025
@puremourning
Copy link
Member

@Mergifyio rebase

Thanks for this! sorry I completely missed it.

@mergify
Copy link
Contributor

mergify bot commented Dec 29, 2025

rebase

✅ Branch has been successfully rebased

Copy link
Member

@puremourning puremourning left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov
Copy link

codecov bot commented Dec 29, 2025

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.73%. Comparing base (fca8f32) to head (3999cf5).
⚠️ Report is 5 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #4294       +/-   ##
===========================================
+ Coverage   75.36%   89.73%   +14.37%     
===========================================
  Files          30       37        +7     
  Lines        2902     4765     +1863     
===========================================
+ Hits         2187     4276     +2089     
+ Misses        715      489      -226     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alxn1
Copy link
Contributor Author

alxn1 commented Dec 30, 2025

Thank you very much!
And i'm so sorry - i'm couldn't imagine that i can break some tests with this little patch.
I'd fixed tests (which was trying to check requests after completion) today, and for now all should be green (i checked it with docker). I hope :)

@puremourning puremourning merged commit 5110798 into ycm-core:master Dec 30, 2025
7 of 8 checks passed
@puremourning
Copy link
Member

Thanks for the PR!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants