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

Refactor `PythonSetup.interpreter_or_constraints()` to take a `compatibility` value rather than a `PythonTarget` #7691

Conversation

Projects
None yet
3 participants
@Eric-Arellano
Copy link
Contributor

commented May 9, 2019

Problem

Our V2 Target API is not fully implemented, including that we do not populate in target adaptors attributes with a default value of None (#7680). This limitation resulted in https://github.com/pantsbuild/pants/pull/7679/files#diff-04a0048c70898e46a42c73225c47b906R58 having to monkey patch the .compatibility field for the target adaptors to get PythonSetup.interpreter_or_constraints() to not raise an AttributeError.

While the underlying issue could be resolved with changing the V2 Target API, we can work around this by changing PythonSetup.interpreter_or_constraints() to no longer require a Target but rather just its .compatibility field.

Solution

Change PythonSetup.interpreter_or_constraints() to have a more direct API of expecting the compatibility field itself.

Not using a deprecation

Neither this function nor its class PythonSetup have a public API. Per our deprecation policy, and confirmed via Slack, it was decided that we do not need a deprecation cycle for this change.

Result

#7680 is unblocked.

Regardless of V2, this is also arguably a better API to only request as a parameter the field we actually need. For example, this makes testing easier because we no longer would need to create a fake PythonTarget.

@stuhood

stuhood approved these changes May 9, 2019

Copy link
Member

left a comment

Thanks!

@benjyw

benjyw approved these changes May 9, 2019

@Eric-Arellano Eric-Arellano merged commit cae058e into pantsbuild:master May 9, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@Eric-Arellano Eric-Arellano deleted the Eric-Arellano:interpreter-constraints-takes-compatibility branch May 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.