-
Notifications
You must be signed in to change notification settings - Fork 61
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
Pin Pip and fix lint e2e tests #201
Conversation
Signed-off-by: Nok <[email protected]>
Signed-off-by: Nok <[email protected]>
Signed-off-by: Nok <[email protected]>
Signed-off-by: Nok <[email protected]>
Signed-off-by: Nok <[email protected]>
Signed-off-by: Nok <[email protected]>
Signed-off-by: Nok <[email protected]>
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.
Thanks @noklam ⭐
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.
🚀 Left a couple of nonimportant comments
def run( | ||
cmd: list | str, split: bool = True, print_output: bool = False, **kwargs: Any | ||
) -> subprocess.CompletedProcess: | ||
"""Run a shell command. | ||
|
||
Args: | ||
cmd: A command string, or a command followed by program | ||
arguments that will be submitted to Popen to run. | ||
|
||
split: Flag that splits command to provide as multiple *args | ||
to Popen. Default is True. | ||
|
||
print_output: If True will print previously captured stdout. | ||
Default is False. | ||
|
||
**kwargs: Extra options to pass to subprocess. | ||
|
||
Example: | ||
:: | ||
"ls" | ||
"ls -la" | ||
"chmod 754 local/file" | ||
|
||
Returns: | ||
Result with attributes args, returncode, stdout and stderr. | ||
|
||
""" | ||
if isinstance(cmd, str) and split: | ||
cmd = shlex.split(cmd) | ||
result = subprocess.run(cmd, input="", capture_output=True, **kwargs) # noqa: PLW1510 | ||
result.stdout = result.stdout.decode("utf-8") | ||
result.stderr = result.stderr.decode("utf-8") | ||
if print_output: | ||
print(result.stdout) | ||
return result | ||
|
||
def call(cmd, env): | ||
res = run(cmd, env=env) | ||
if res.returncode: | ||
print(res.stdout) | ||
print(res.stderr) | ||
assert 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.
Is this more or less check_output
? https://docs.python.org/3/library/subprocess.html#subprocess.check_output
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.
This chunk of code is borrowed from Kedro repo.
"-m", | ||
"pip", | ||
"install", | ||
"-U", |
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.
nit: No need to do -U
if installing a specific version?
Merging anyway, we can address the comments later |
Motivation and Context
kedro lint
Noted pip version is set twice. Previously pip version are sometimes in 23.3 or 21 depending on the OS/tests combo.
How has this been tested?
Checklist