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

dduvall created this revision.Feb 13 2018, 8:31 PM
Restricted Application added a reviewer: Release-Engineering-Team. · View Herald TranscriptFeb 13 2018, 8:31 PM
Restricted Application added a project: Release-Engineering-Team. · View Herald Transcript
dduvall requested review of this revision.Feb 13 2018, 8:31 PM

A first go at this.

dduvall updated this revision to Diff 2559.Feb 13 2018, 10:59 PM

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

dduvall updated this revision to Diff 2560.Feb 13 2018, 11:29 PM

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

dduvall updated this revision to Diff 2564.Feb 14 2018, 6:50 PM
dduvall edited the summary of this revision. (Show Details)

Improved the commit message

dduvall updated this revision to Diff 2565.Feb 14 2018, 6:52 PM
dduvall edited the summary of this revision. (Show Details)

Got a little backtick happy

demon added inline comments.Feb 14 2018, 6:56 PM
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...

dduvall added inline comments.Feb 14 2018, 7:13 PM
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.

thcipriani accepted this revision.Mar 5 2018, 10:27 PM

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
dduvall updated this revision to Diff 2615.Mar 6 2018, 12:22 AM

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

thcipriani accepted this revision.Mar 6 2018, 3:07 PM

RequirementsByDir update lgtm.

config/python.go
146

Nice, lgtm :)

This revision was automatically updated to reflect the committed changes.