Fix "ValueError: too many values to unpack" when parsing interpreter versions.

Review Request #3411 - Created Feb. 2, 2016 and submitted

Information
Kris Wilson
pants
kwlzn/interpreter_crash
2038, 2872
Reviewers
pants-reviews
benjyw, jsirois, nhoward_tw, patricklaw

When encountering failures translating setuptools and/or wheel during interpreter bootstrap, the current interpreter cache setup logic can leave temporary-named directories behind (e.g. CPython-2.7.8.tmp.52b0f0a2542a44bfb16f247ec9f6215a). This causes issues during subsequent runs when trying to parse interpreter versions from the cache.

  • Convert safe_concurrent_create to a context manager (with a slight rename to align with the contextmanager context).
  • Safeguard tmp path conversions with a finally block.
  • Misc cleanup.
  • Test coverage.

Closes #2038

CI is green @ https://travis-ci.org/pantsbuild/pants/builds/106402640

Verified interpreter_cache repro test fails pre-fix both in CI and locally.

Issues

  • 1
  • 3
  • 0
  • 4
Description From Last Updated
Definitely a better implementation, but why the name change? We generally name functions as imperatives ("do this") rather than as ... Benjy Weinberger Benjy Weinberger
Nick Howard (Twitter)
Yi Cheng
Patrick Lawson
Kris Wilson
Nick Howard (Twitter)
Kris Wilson
Review request changed

Status: Closed (submitted)

Change Summary:

thanks Nick, Yi & Patrick! this is in @ d4e95750f94b046d40e252ff8a4aada43000f68e

Benjy Weinberger

   
src/python/pants/util/dirutil.py (Diff revision 2)
 
 

Definitely a better implementation, but why the name change? We generally name functions as imperatives ("do this") rather than as nouns ("the thing that will be done").

E.g., in this very file we have safe_concurrent_rename, not safe_concurrent_renaming.

Or if you want a contextmanager example, we open files like this:

with open(path) as f:

not like this:

with opened(path) as f:

Loading...