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

Fixed HTTPConnection leaking #542

Merged
merged 2 commits into from
Oct 2, 2018
Merged

Fixed HTTPConnection leaking #542

merged 2 commits into from
Oct 2, 2018

Conversation

jared-mackey
Copy link
Contributor

Closing the connection after use to ensure sockets do not leak.

Fixes #541

Closing the connection after use to ensure sockets do not leak.
@palazzem
Copy link

Thanks @mackeyja92 for the contribution! We'll take a look to test properly the change. I'll reach you once we schedule this PR for the next release!

Copy link
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

This looks good to me, we should see if we can add some sort of a regression test for this so that we don't make the same mistake in the future.

@jared-mackey
Copy link
Contributor Author

@Kyle-Verhoog You could run your tests with the python argument -W default to get warnings printed out during testing.

@palazzem
Copy link

If it's a warning, can we detect them via warnings module? (same as:

def test_patch_signals_connect(self):
# calling `patch_task` enables instrumentation globally
# while raising a Deprecation warning
with warnings.catch_warnings(record=True) as w:
warnings.simplefilter('always')
@patch_task
@self.app.task
def fn_task():
return 42
ok_(len(w) == 1)
ok_(issubclass(w[-1].category, DeprecationWarning))
ok_('patch(celery=True)' in str(w[-1].message))
).

If it's possible, it's enough to check that no warnings are raised.

@palazzem palazzem modified the milestones: 0.13.1, 0.13.2 Aug 29, 2018
@labbati labbati removed this from the 0.13.2 milestone Sep 25, 2018
@Kyle-Verhoog Kyle-Verhoog added this to the 0.15.0 milestone Oct 2, 2018
@Kyle-Verhoog Kyle-Verhoog merged commit b05208b into DataDog:master Oct 2, 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.

None yet

4 participants