Skip to content

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

Merged
nerdvegas merged 1 commit into
AcademySoftwareFoundation:masterfrom
alexey-pelykh:bugfix/ResolvedContext__get_resolved_package
Jun 22, 2022
Merged

ResolvedContext.get_resolved_package() should return None for failure#1325
nerdvegas merged 1 commit into
AcademySoftwareFoundation:masterfrom
alexey-pelykh:bugfix/ResolvedContext__get_resolved_package

Conversation

@alexey-pelykh

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

Copy link
Copy Markdown
Contributor

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

linux-foundation-easycla Bot commented Jun 14, 2022

Copy link
Copy Markdown

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

ColinKennedy commented Jun 14, 2022

Copy link
Copy Markdown

@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
Copy Markdown
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
Copy Markdown

Music to my ears!

@nerdvegas

nerdvegas commented Jun 15, 2022 via email

Copy link
Copy Markdown
Contributor

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
Copy Markdown
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