Page MenuHomePhabricator

Support Python projects
ClosedPublic

Authored by dduvall on Feb 13 2018, 8:31 PM.

Details

Maniphest Tasks
T186545: Blubber should support python/tox
Reviewers
thcipriani
hashar
demon
Group Reviewers
Release-Engineering-Team
Commits
rGBLBR6896e655eb5c: Support Python projects
Patch without arc
git checkout -b D976 && curl -L https://phabricator.wikimedia.org/D976?download=true | git apply
Summary

A new root and variant python config field is provided with two new
fields below, version and requirements.

The former, version, should specify the Python executable to use when
executing related package installation commands and ostensibly the same
executable that will be used to run the application.

The latter, requirements, should specify all pip requirements files
such that a compiler that supports layered filesystems (e.g. Docker) can
output separate instructions that will invalidate cache layers for
changes to those files independently of changes to the rest of the
codebase.

Python related instructions will be generated only if either version
or requirements are given.

Fixes T186545

Test Plan

Run go test ./....

Diff Detail

Repository
rGBLBR Blubber
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fix linter warnings and export/test PythonConfig.RequirementsByDir() and PythonConfig.RequirementsArgs()

Fix merge behavior around PythonConfig.Version and consider Version for whether to manage Python in environment

dduvall edited the summary of this revision. (Show Details)

Improved the commit message

dduvall edited the summary of this revision. (Show Details)

Got a little backtick happy

config/python.go
69

Is there a reason we're installing pip via easy_install and not the apt package? I've never had any problem with the latter.

70

Same question here, really. Although with these, we might want versions not yet in debian's apt repos...

config/python.go
69

I've run into some problems with older versions of pip in scap's Dockerfile.ci (which is what this was based on) but I don't recall exactly what they were. Generally I think we'll run into fewer problems using the latest versions but maybe that's too trusting of maintainers.

70

Same as above. An old setuptools caused issues just recently for installation of scap's dependencies, a "produced metadata for project name unknown" error similar to this one.

Looks sane to me. Found one thing that might confuse some folks noted inline.

config/python.go
174

function seems a little klugy. Something like "./requirements.txt" results in stuff in the Dockerfile that end up looking like mkdir "-p" "././" and COPY ["./requirements.txt", "./test-requirements.txt", "././"] which surprising behavior.

Could maybe use filepath.Clean first?

This revision is now accepted and ready to land.Mar 5 2018, 10:27 PM

Removed python.reqDirname in favor of path.Dir and python.RequirementsByDir now cleans paths

RequirementsByDir update lgtm.

config/python.go
146

Nice, lgtm :)

This revision was automatically updated to reflect the committed changes.