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

Use dedicated requirements.txt for contrib.python.checks.checker #7982

Merged
merged 3 commits into from Jun 30, 2019

Conversation

Eric-Arellano
Copy link
Contributor

@Eric-Arellano Eric-Arellano commented Jun 29, 2019

Problem

contrib.python.checks.checker must keep support for Python 2, e.g. using six, which we will soon drop from the rest of the codebase.

Likewise, the module uses pycodestyle and pyflakes, neither of which are used anywhere else in the project.

We would like a way to provide requirements for specifically this module and to not pollute the rest of the project in doing so.

Solution

Introduce a dedicated 3rdparty/requirements.txt for the module, per https://www.pantsbuild.org/3rdparty_py.html#3rdpartypython.

We also indicate pants_venv should install these dependencies locally and release.sh should build 3rd party wheels for this so that the released PEX continues to work for the contrib package.

@Eric-Arellano
Copy link
Contributor Author

Running this results in the below error:

 File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/bin/pants_exe.py", line 36, in main
    PantsRunner(exiter, start_time=start_time).run()
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/bin/pants_runner.py", line 87, in run
    options_bootstrapper=options_bootstrapper
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/bin/local_pants_runner.py", line 134, in create
    options_bootstrapper=options_bootstrapper,
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/bin/local_pants_runner.py", line 83, in parse_options
    build_config = BuildConfigInitializer.get(options_bootstrapper)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/init/options_initializer.py", line 35, in get
    cls._cached_build_config = cls(options_bootstrapper).setup()
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/init/options_initializer.py", line 66, in setup
    self._bootstrap_options.backend_packages
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/init/options_initializer.py", line 55, in _load_plugins
    return load_backends_and_plugins(plugins, working_set, backend_packages)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/init/extension_loader.py", line 32, in load_backends_and_plugins
    load_build_configuration_from_source(build_configuration, backends)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/init/extension_loader.py", line 113, in load_build_configuration_from_source
    load_backend(build_configuration, backend_package)
  File "/Users/eric/DocsLocal/code/projects/pants/src/python/pants/init/extension_loader.py", line 131, in load_backend
    .format(backend=backend_module, error=e))

Exception message: Failed to load the pants.contrib.python.checks.register backend: No module named 'pycodestyle'

@jsirois does this overall approach look right?

@Eric-Arellano Eric-Arellano changed the title WIP: Use dedicated requirements.txt for contrib.python.checks.checker Use dedicated requirements.txt for contrib.python.checks.checker Jun 29, 2019
@Eric-Arellano Eric-Arellano marked this pull request as ready for review June 29, 2019 05:42
@Eric-Arellano
Copy link
Contributor Author

Never mind, got it working :) Was missing an entry in build-support/pants_venv.

# Copyright 2019 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

python_requirements()
Copy link
Sponsor Contributor

Choose a reason for hiding this comment

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

Since all these deps are used in just one place, together, you could forgo the requirements.txt and the separate targets per requirement, and just have

python_requirement_library(
  requirements = [
    python_requirement('six>=1.9.0,<2')
    python_requirement('pycodestyle==2.4.0'),
    python_requirement('pyflakes==2.0.0'),
  ]
)

Then you can depend on all of them at once by depending on

contrib/python/src/python/pants/contrib/python/checks/checker/3rdparty

Which would be more succinct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to use this approach, but the downside is that virtual_env and release.sh won't know to install / build wheels for those 3 dependencies, which we need them to do.

@Eric-Arellano Eric-Arellano merged commit c26f374 into pantsbuild:master Jun 30, 2019
@Eric-Arellano Eric-Arellano deleted the checker-requirements branch June 30, 2019 17:20
@Eric-Arellano Eric-Arellano mentioned this pull request Jul 1, 2019
Eric-Arellano added a commit that referenced this pull request Jul 1, 2019
However, `contrib.python.checks.checker` does still need `six`, which was setup in #7982 to be isolated from this change.
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.

None yet

2 participants