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 for a regression which broke sensors which rely on select.poll() functionality #4118

Merged
merged 9 commits into from
May 9, 2018

Conversation

Kami
Copy link
Member

@Kami Kami commented May 9, 2018

Background

In v2.7.0 we upgraded eventlet to the latest stable version. That stable version removed select.poll() which was available in previous releases (but it was blocking) to make it harder for users to "accidentally" use blocking code.

That change broke some sensors which rely on select.poll() functionality.

Users would see an error similar to one below:

2018-05-09 07:57:29,425 ERROR [-] AttributeError
2018-05-09 07:57:29,425 ERROR [-] : 
2018-05-09 07:57:29,425 ERROR [-] 'module' object has no attribute 'poll'
2018-05-09 07:57:29,425 ERROR [-] 

Proposed solution

This PR adds select module which includes blocking version of poll() back to sys.modules so it can be used by the sensors which need it.

The behavior will now be the same as in releases prior to v2.7.0. Blocking code is not great and ideally we could avoid it where we can, but sadly we have no control over 3rd party libraries and integrations which use that blocking code construct.

Keep in mind, that each sensor is running in a separate and dedicated process so blocking inside that process it not that big of deal (compared to blocking in a main service process which would be a no go).

Related issues #3968 #4007 #4094

Resolves #4094

Kami added 6 commits May 8, 2018 19:24
and allow it to be used outside the nose tests context.
This way it fixes regression introduced in 2.7.0 and sensors which rely
on subprocess.poll() (select.poll()) still work in the same manner as
they did before v2.7.0.
@Kami Kami added this to the 2.7.2 milestone May 9, 2018
@enykeev enykeev self-requested a review May 9, 2018 10:24
@schreyack
Copy link

I can confirm that these changes fix the issue for the git sensor.

@Kami
Copy link
Member Author

Kami commented May 9, 2018

@schreyack Great - thanks for confirming :)

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.

3 participants