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

ResolvedContext.get_resolved_package() should return None for failure #1325

Conversation

alexey-pelykh
Copy link
Contributor

@alexey-pelykh alexey-pelykh commented Jun 14, 2022

Otherwise for rez-pip --python-version 1.2.3 -i pandas with non-existent python version, the following stacktrace will happen:

Traceback (most recent call last):
  File "C:\Users\user\AppData\Local\Programs\Python\Python37\lib\runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "C:\Users\user\AppData\Local\Programs\Python\Python37\lib\runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "C:\Users\user\rez\Scripts\rez\rez-pip.exe\__main__.py", line 7, in <module>
  File "c:\users\user\rez\lib\site-packages\rez\cli\_entry_points.py", line 183, in run_rez_pip
    return run("pip")
  File "c:\users\user\rez\lib\site-packages\rez\cli\_main.py", line 191, in run
    returncode = run_cmd()
  File "c:\users\user\rez\lib\site-packages\rez\cli\_main.py", line 183, in run_cmd
    return func(opts, opts.parser, extra_arg_groups)
  File "c:\users\user\rez\lib\site-packages\rez\cli\pip.py", line 73, in command
    extra_args=opts.extra)
  File "c:\users\user\rez\lib\site-packages\rez\pip.py", line 264, in pip_install_package
    py_exe, context = find_pip(pip_version, python_version)
  File "c:\users\user\rez\lib\site-packages\rez\pip.py", line 95, in find_pip
    python_version, pip_version=version
  File "c:\users\user\rez\lib\site-packages\rez\pip.py", line 203, in find_pip_from_context
    py_exe = find_python_in_context(context)
  File "c:\users\user\rez\lib\site-packages\rez\pip.py", line 142, in find_python_in_context
    python_package = context.get_resolved_package("python")
  File "c:\users\user\rez\lib\site-packages\rez\resolved_context.py", line 441, in get_resolved_package
    pkgs = [x for x in self._resolved_packages if x.name == name]
TypeError: 'NoneType' object is not iterable

@alexey-pelykh alexey-pelykh requested a review from nerdvegas as a code owner June 14, 2022 16:22
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jun 14, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: alexey-pelykh / name: Alexey Pelykh (6b32be8)

@alexey-pelykh alexey-pelykh force-pushed the bugfix/ResolvedContext__get_resolved_package branch from ee2f3f2 to ed8be79 Compare June 14, 2022 16:22
@ColinKennedy
Copy link

ColinKennedy commented Jun 14, 2022

@alexey-pelykh what is the outcome from rez-pip once your PR is merged? I'd really like / expect rez-pip to give a nice message like "No valid version for pandas could be found" or something. Can we make sure prior to merge that we get a similar message?

@alexey-pelykh
Copy link
Contributor Author

@ColinKennedy it will fallback to rez-bound python with warning instead of crashing miserably

18:14:54 WARNING  Found no pip in any python and/or pip rez packages!
18:14:54 WARNING  Falling back to pip installed in rez own virtualenv:
18:14:54 WARNING         pip: 22.0.4 (/Users/opelykh/rez/lib/python3.9/site-packages/pip/__init__.py)
18:14:54 WARNING      python: 3.9.12 (/Users/opelykh/rez/bin/python)
18:14:54 INFO     Installing 'pandas' with pip taken from '/Users/opelykh/rez/bin/python'

@alexey-pelykh alexey-pelykh force-pushed the bugfix/ResolvedContext__get_resolved_package branch from ed8be79 to 6b32be8 Compare June 14, 2022 16:35
@ColinKennedy
Copy link

Music to my ears!

@nerdvegas
Copy link
Contributor

nerdvegas commented Jun 15, 2022 via email

Signed-off-by: Alexey Pelykh <alexey.pelykh@gmail.com>
@alexey-pelykh alexey-pelykh force-pushed the bugfix/ResolvedContext__get_resolved_package branch from 6b32be8 to 96dd363 Compare June 15, 2022 05:20
@alexey-pelykh
Copy link
Contributor Author

@nerdvegas done

@nerdvegas nerdvegas merged commit 7235e87 into AcademySoftwareFoundation:master Jun 22, 2022
@alexey-pelykh alexey-pelykh deleted the bugfix/ResolvedContext__get_resolved_package branch June 22, 2022 11:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants