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

Using --jobs affects monkey patching detection #502

Closed
pylint-bot opened this issue Mar 25, 2015 · 8 comments
Closed

Using --jobs affects monkey patching detection #502

pylint-bot opened this issue Mar 25, 2015 · 8 comments
Labels
Bug 🪲 Duplicate 🐫 Duplicate of an already existing issue multiprocessing

Comments

@pylint-bot
Copy link

Originally reported by: Pavel Roskin (BitBucket: pavel_roskin)


first.py:

import sys
sys.foo = 0

second.py:

import sys
sys.foo

pylint -E first.py second.py

No output

pylint -E --jobs=2 first.py second.py

************* Module second
E:  2, 0: Module 'sys' has no 'foo' member (no-member)

I'm actually fine if pylint reports that error in every case, as long as there is no import first in second.py before sys.foo is used. The original code that triggered the error message is overengineered and needs fixing.


@pylint-bot
Copy link
Author

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):


It's not just with multiple jobs, it fails by reversing the order of the files in the CLI command as well. It doesn't report in the first case, because pylint uses a global cache of modules, astroid.manager.AstroidManager.astroid_cache, so when it processes second.py, sys is already present in this cache and it's already patched and pylint just grabs it from there. In the second case, sys is not there yet and it fails. When --jobs are involved, there's no guaranteed order, plus the fact that each job is a separate process. This suggests a possible fix, to clear the cache after each processed file, but that hits the performance, since the modules will have to be reprocessed over and over again for each new file. I'm inclined to close this as wont-fix, since it's such an edge case.

@pylint-bot
Copy link
Author

Original comment by Pavel Roskin (BitBucket: pavel_roskin):


I think many people would prefer pylint to run ten times slower but detect 10% more issues. I'd rather run pylint overnight than release code where pylint failed to find a problem. Of course, slowing down pylint could also discourage people from using it in more casual situations (e.g. checking a trivial code change before committing it).

If running pylint on every file individually would produce more reliable results at the expense of processing time, then maybe it could be an option? With that option enabled, all caches would be cleaned when pylint starts processing a new file.

By the way, a reliable --jobs implementation would offset some of the additional time. Especially if --jobs is set by default to the number of CPUs.

@pylint-bot
Copy link
Author

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):


Well, the question is if detecting monkey patching is a real problem. We'll have to investigate what other side effects the cleaning of cache has after all.

@pylint-bot
Copy link
Author

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):


An option sounds good.

@pylint-bot
Copy link
Author

Original comment by Pavel Roskin (BitBucket: pavel_roskin):


In my case, it wasn't a new information. It was already clear that defining __builtins__._ was a bad approach that needed replacement.

However, I know that running pylint on individual files produces more data than running it on the whole module. I'll see if pylint would find anything useful by checking each file separately. That would produce a more realistic motivation for a possible option.

@pylint-bot
Copy link
Author

Original comment by Claudiu Popa (BitBucket: PCManticore, GitHub: @PCManticore):


Ah, that sounds perfect for additional-builtins option.

msuozzo pushed a commit to msuozzo/pylint that referenced this issue Feb 18, 2022
~~Based on branch for PR pylint-dev#500 -- I will rebase after that PR merges.~~

Closes pylint-dev#367.
    
Supersedes PR pylint-dev#497.
@Pierre-Sassoulas
Copy link
Member

This bug, while ancient, is still reproducible in latest code on 2.15.0-dev0.

@Pierre-Sassoulas Pierre-Sassoulas added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jul 1, 2022
@Pierre-Sassoulas
Copy link
Member

Closing as duplicate of #374

@Pierre-Sassoulas Pierre-Sassoulas added Duplicate 🐫 Duplicate of an already existing issue and removed Needs PR This issue is accepted, sufficiently specified and now needs an implementation labels Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug 🪲 Duplicate 🐫 Duplicate of an already existing issue multiprocessing
Projects
None yet
Development

No branches or pull requests

2 participants