[4.0.x] Make manager required in AstroidBuilder/InspectBuilder

[jacobtylerwalls]

Type of Changes

	Type
✓	🔨 Refactoring

Description

Closes #251

Remove delayed imports in AstroidManager by requiring AstroidManager to be provided to AstroidBuilder/InspectBuilder.

A couple new delayed imports became necessary, but they're in helpers like parse() and _astroid_bootstrapping(). We could address those by:

• Requiring AstroidManager for parse() calls also.
• Moving _astroid_bootstrapping() to manager.py.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.84%. Comparing base (0883b04) to head (c9399bb).
Report is 89 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2313      +/-   ##
==========================================
- Coverage   92.85%   92.84%   -0.01%     
==========================================
  Files          94       94              
  Lines       11056    11043      -13     
==========================================
- Hits        10266    10253      -13     
  Misses        790      790              

Flag	Coverage Δ
linux	92.65% <100.00%> (-0.01%)	⬇️
pypy	90.99% <100.00%> (-0.01%)	⬇️
windows	92.43% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
astroid/brain/brain_nose.py	95.12% <100.00%> (ø)
astroid/builder.py	94.14% <100.00%> (+0.02%)	⬆️
astroid/interpreter/_import/spec.py	97.40% <100.00%> (ø)
astroid/manager.py	88.79% <100.00%> (-0.24%)	⬇️
astroid/raw_building.py	94.93% <100.00%> (-0.02%)	⬇️
astroid/rebuilder.py	98.43% <100.00%> (-0.03%)	⬇️

[Pierre-Sassoulas]

This one bring a lot of joy. Were you able to use nodes.X instead of X thanks to the fixed circular imports ?

[jacobtylerwalls]

I didn't have a choice, because the second import statement [from nodes import ...] was failing because nodes was now partially installed at that point. I think before, it was an accident that nodes was fully initialized already.

[DanielNoord]

Although really in favour, this do introduce more global state for AstroidManager(), see also #2048.

As long as AstroidManager is a singleton we'll keep accessing shared memory in multiprocessing scenarios. Ideally, to me, we would change it in 4.x so that the manager needs to be instantiated.

[jacobtylerwalls]

Although really in favour, this do introduce more global state for AstroidManager(), see also #2048.

Getting back up to speed on this to see if I can get it in for 4.0. @DanielNoord can you remind me what you meant by "more global state"? I'm not sure I follow.

[DanielNoord]

From my testing with multiprocessing on of the issues is that the cache of all nodes is stored on the manager, which means that we have a lot of dependency on this one shared object. Since it is global, and not passed or instantiated in pylint, it is quite hard to extract that dependency and (for example) give every process its own cache.

I'm fine with this PR, but I do think at some point we need to look into making the manager something that is instantiated by the process that wants to do inference.