-
Notifications
You must be signed in to change notification settings - Fork 28
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
Add sys.path
to collection paths
#318
Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ScanSysPath(isolated=True, scan=True, expected=False), | ||
ScanSysPath(isolated=True, scan=False, expected=False), | ||
ScanSysPath(isolated=False, scan=True, expected=True), | ||
ScanSysPath(isolated=False, scan=False, expected=False), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on the result, if scan
is False, no matter what value isolated
is, the expected
is always False.
If scan
is True
, I think the expected
should be True. It's not related to isolated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if not self.isolated and self.config.collections_scan_sys_path:
this one confirms that if isolated and scan sys is true we still don't inject
ScanSysPath(isolated=True, scan=False, expected=False),
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need add isolated in condition.
IIUC, this is a common issue.
No matter it is isolated, the sys.path is required.
Let me know what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need @ssbarnea's input here, since lint uses isolated, I think he will better know the impact... Maybe a sperate PR to remove the isolated logic and see what he says?
I do not have a good understanding of what isolated is intended to accomplish :/
Related: ansible/molecule#4017
If not isolated and COLLECTION_SCAN_SYS_PATH is true, add the sys.paths to collection paths.
This is intended to mimic the functionality of the collection loader