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

[Internal] Refactor how PythonToolBase exposes requirements and interpreter constraints #12356

Merged
merged 1 commit into from
Jul 15, 2021

Conversation

Eric-Arellano
Copy link
Contributor

In a followup, the new PythonToolBase.pex_requirements will have the logic for how to load a lockfile:

if docformatter.lockfile == "<none>":
requirements = PexRequirements(docformatter.all_requirements)
elif docformatter.lockfile == "<default>":
requirements = PexRequirements(
file_content=FileContent(
"docformatter_default_lockfile.txt",
importlib.resources.read_binary(
"pants.backend.python.lint.docformatter", "lockfile.txt"
),
)
)
else:
requirements = PexRequirements(
file_path=docformatter.lockfile,
file_path_description_of_origin="the option `[docformatter].experimental_lockfile`",
)

This will allow us to DRY setup of lockfiles.

[ci skip-rust]
[ci skip-build-wheels]

…erpreter constraints

# Rust tests and lints will be skipped. Delete if not intended.
[ci skip-rust]

# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
Copy link
Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent.

Comment on lines +78 to +87
@property
def pex_requirements(self) -> PexRequirements:
"""The requirements to be used when installing the tool.

If the tool supports lockfiles, the returned type will install from the lockfile rather than
`all_requirements`.
"""
# TODO(#11898): move lockfile option registration into PythonToolBase, and update this to
# set the PexRequirements appropriately.
return PexRequirements(self.all_requirements)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@Eric-Arellano Eric-Arellano merged commit a5584f2 into pantsbuild:main Jul 15, 2021
@Eric-Arellano Eric-Arellano deleted the pex-reqs branch July 15, 2021 00:13
@wisechengyi wisechengyi mentioned this pull request Jul 17, 2021
wisechengyi added a commit that referenced this pull request Jul 17, 2021
### Internal

* [internal] Manually fix Black lockfile to handle interpreter constraints ([#12366](#12366))
* Revert "Prefix the entire setup.py chroot. (#12359)" ([#12370](#12370))
* [internal] Cache native client binary in CI ([#12355](#12355))
* Prefix the entire setup.py chroot. ([#12359](#12359))
* [internal] Fix AWS CLI breaking due to Python 2 usage ([#12364](#12364))
* [Internal] Add `git_url()` helper to `docutil.py` ([#12352](#12352))
* [Internal] Refactor how `PythonToolBase` exposes requirements and interpreter constraints ([#12356](#12356))
* [internal] Add experimental per-tool lockfiles with Docformatter as an example ([#12346](#12346))
* Remove Pants's dpeendency on `requests` ([#12348](#12348))
* [internal] Remove `TwoStepPex` abstraction ([#12343](#12343))
* Add psycopg2-binary to default module mapping ([#12339](#12339))
* [internal] Upgrade toolchain pants plugin to 0.13.1 ([#12338](#12338))
* Add an API to coarsen/partition Targets by their cycles ([#12251](#12251))
* Prepare 2.5.1 ([#12329](#12329))
* Bootstrap fewer JVM versions in Coursier/javac tests to hopefully reduce CI flakiness ([#12325](#12325))
* Native client respects `--concurrent`. ([#12324](#12324))
* Add client lib tests. ([#12322](#12322))
* Special case enum option parse failures. ([#12281](#12281))
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.

2 participants