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

(Backport 53167) loader: support extra modules in sys.path #53440

Closed
wants to merge 5 commits into from

Conversation

aplanas
Copy link
Contributor

@aplanas aplanas commented Jun 11, 2019

What does this PR do?

The LazyLoader class populate the sys.path array to add the path
that contains the Python module that will be executed. If before
this point sys.path gets extended (like when globally the extmods
utils directory in added in some places, like after a sync) the
code can find all the dependencies.

This patch adds a new parameter to the LazyLoader class,
extra_module_dirs, that will populate locally the sys.path before
executing the the module.

This can be used to guarantee that in utils (and other modules) will be
available to the modules and states in any circumstances and Python
process.

Tests written?

Yes

(backport #53167)

@aplanas aplanas requested a review from a team as a code owner June 11, 2019 08:54
Copy link
Contributor

@waynew waynew left a comment

Choose a reason for hiding this comment

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

Pretty sure the code is fine, and the test failure is due to a dependency failing to get installed.

We need to get that sorted out so the tests all pass.

@waynew waynew added pending-changes The pull request needs additional changes before it can be merged and removed pending-changes The pull request needs additional changes before it can be merged labels Jun 13, 2019
@cmcmarrow cmcmarrow self-requested a review June 14, 2019 20:12
@waynew
Copy link
Contributor

waynew commented Jun 27, 2019

re-run all

@waynew
Copy link
Contributor

waynew commented Jul 3, 2019

@aplanas is there any way that these git test failures

are related to this change? It seems unlikely to me without further digging, and weird that it's just these couple of platforms on which it's failing... 🤷‍♂

@aplanas
Copy link
Contributor Author

aplanas commented Jul 22, 2019

@waynew I rebased the code, to relaunch the CI

(cherry picked from commit bb798685d29261f5dc290ba08694214be2204a7a)
(cherry picked from commit f7b0c8b68470256de225bee7343ff21c9af8cc60)
(cherry picked from commit 5a8a0ce3c3f1d27f7ae55167df8c98d021873f9e)
The LazyLoader class populate the sys.path array to add the path
that contains the Python module that will be executed.  If before
this point sys.path gets extended (like when globally the extmods
utils directory in added in some places, like after a sync) the
code can find all the dependencies.

This patch adds a new parameter to the LazyLoader class,
extra_module_dirs, that will populate locally the sys.path before
executing the the module.

This can be used to guarantee that in utils (and other modules) will be
available to the modules and states in any circumstances and Python
process.

(cherry picked from commit 670974f9d7e95a030a1429b3abe2e75873ebb3b6)
This reverts commit 7e82f3e.

(cherry picked from commit e0e5c282583db7fd3ba4d21bcfadf30943f04ab7)
@aplanas aplanas closed this Oct 14, 2019
@aplanas aplanas deleted the backport_53167 branch October 14, 2019 12:48
@aplanas
Copy link
Contributor Author

aplanas commented Oct 14, 2019

Rebased on master as part of #53167

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.

5 participants