Skip to content

Commit

Permalink
Port Django migrations inference away from `PythonDependencyVisitorRe…
Browse files Browse the repository at this point in the history
…quest` (#19008)

This is the only in-repo plugin using `PythonDependencyVisitorRequest`,
meaning when the rust parser is the new only implementation
`PythonDependencyVisitorRequest` and plumbing can be removed (sooner if
we refactor)

Implementation:
- Switched to running a dedicated process for scraping, instead of
piggybacking off of the existing one
- Only scrape from files directly under a `migrations` directory
  • Loading branch information
thejcannon authored May 17, 2023
1 parent 9d845f0 commit f9188ab
Show file tree
Hide file tree
Showing 7 changed files with 174 additions and 119 deletions.
2 changes: 1 addition & 1 deletion src/python/pants/backend/python/framework/django/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@
# Licensed under the Apache License, Version 2.0 (see LICENSE).


python_sources(dependencies=["src/python/pants/backend/python/framework/django/scripts"])
python_sources()

python_tests(name="tests")
Original file line number Diff line number Diff line change
@@ -1,49 +1,122 @@
# Copyright 2023 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from dataclasses import dataclass
import json
from pathlib import PurePath

from pants.backend.python.dependency_inference.parse_python_dependencies import (
PythonDependencyVisitor,
PythonDependencyVisitorRequest,
get_scripts_digest,
ParsedPythonAssetPaths,
ParsedPythonDependencies,
ParsedPythonImportInfo,
ParsedPythonImports,
)
from pants.backend.python.dependency_inference.rules import (
ImportOwnerStatus,
PythonImportDependenciesInferenceFieldSet,
ResolvedParsedPythonDependencies,
ResolvedParsedPythonDependenciesRequest,
)
from pants.backend.python.framework.django.detect_apps import DjangoApps
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import EntryPoint
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex import PexRequest, VenvPex, VenvPexProcess
from pants.core.util_rules.source_files import SourceFilesRequest
from pants.core.util_rules.stripped_source_files import StrippedSourceFiles
from pants.engine.fs import CreateDigest, FileContent
from pants.engine.internals.native_engine import Digest, MergeDigests
from pants.engine.internals.native_engine import Digest
from pants.engine.internals.selectors import Get
from pants.engine.process import ProcessResult
from pants.engine.rules import collect_rules, rule
from pants.engine.target import InferDependenciesRequest, InferredDependencies
from pants.engine.unions import UnionRule
from pants.util.frozendict import FrozenDict
from pants.util.logging import LogLevel
from pants.util.resources import read_resource


@dataclass(frozen=True)
class DjangoDependencyVisitorRequest(PythonDependencyVisitorRequest):
pass
class InferDjangoDependencies(InferDependenciesRequest):
infer_from = PythonImportDependenciesInferenceFieldSet


_scripts_package = "pants.backend.python.framework.django.scripts"
_visitor_resource = "scripts/dependency_visitor.py"


@rule
async def django_parser_script(
_: DjangoDependencyVisitorRequest,
request: InferDjangoDependencies,
python_setup: PythonSetup,
django_apps: DjangoApps,
) -> PythonDependencyVisitor:
django_apps_digest = await Get(
Digest, CreateDigest([FileContent("apps.json", django_apps.to_json())])
) -> InferredDependencies:
source_field = request.field_set.source
# NB: This doesn't consider https://docs.djangoproject.com/en/4.2/ref/settings/#std-setting-MIGRATION_MODULES
if not PurePath(source_field.file_path).match("migrations/*.py"):
return InferredDependencies([])

stripped_sources = await Get(
StrippedSourceFiles, SourceFilesRequest([request.field_set.source])
)
assert len(stripped_sources.snapshot.files) == 1

file_content = FileContent("__visitor.py", read_resource(__name__, _visitor_resource))
visitor_digest = await Get(Digest, CreateDigest([file_content]))
venv_pex = await Get(
VenvPex,
PexRequest(
output_filename="__visitor.pex",
internal_only=True,
main=EntryPoint("__visitor"),
interpreter_constraints=InterpreterConstraints.create_from_compatibility_fields(
[request.field_set.interpreter_constraints], python_setup=python_setup
),
sources=visitor_digest,
),
)
process_result = await Get(
ProcessResult,
VenvPexProcess(
venv_pex,
argv=[stripped_sources.snapshot.files[0]],
description=f"Determine Django app dependencies for {request.field_set.address}",
input_digest=stripped_sources.snapshot.digest,
level=LogLevel.DEBUG,
),
)
scripts_digest = await get_scripts_digest(_scripts_package, ["dependency_visitor.py"])
digest = await Get(Digest, MergeDigests([django_apps_digest, scripts_digest]))
return PythonDependencyVisitor(
digest=digest,
classname=f"{_scripts_package}.dependency_visitor.DjangoDependencyVisitor",
env=FrozenDict({}),
# See in script for where we explicitly encoded as utf8. Even though utf8 is the
# default for decode(), we make that explicit here for emphasis.
process_output = process_result.stdout.decode("utf8") or "{}"
modules = [
"{}.migrations.{}".format(django_apps.label_to_name[label], migration)
for label, migration in json.loads(process_output)
if label in django_apps.label_to_name
]
resolve = request.field_set.resolve.normalized_value(python_setup)

resolved_dependencies = await Get(
ResolvedParsedPythonDependencies,
ResolvedParsedPythonDependenciesRequest(
request.field_set,
ParsedPythonDependencies(
ParsedPythonImports(
(module, ParsedPythonImportInfo(0, False)) for module in modules
),
ParsedPythonAssetPaths(),
),
resolve,
),
)

return InferredDependencies(
sorted(
address
for result in resolved_dependencies.resolve_results.values()
if result.status in (ImportOwnerStatus.unambiguous, ImportOwnerStatus.disambiguated)
for address in result.address
)
)


def rules():
return [
UnionRule(PythonDependencyVisitorRequest, DjangoDependencyVisitorRequest),
*collect_rules(),
UnionRule(InferDependenciesRequest, InferDjangoDependencies),
]
Original file line number Diff line number Diff line change
Expand Up @@ -8,135 +8,108 @@
import pytest

from pants.backend.python.dependency_inference import parse_python_dependencies
from pants.backend.python.dependency_inference.parse_python_dependencies import (
ParsedPythonDependencies,
)
from pants.backend.python.dependency_inference.parse_python_dependencies import (
ParsedPythonImportInfo as ImpInfo,
)
from pants.backend.python.dependency_inference.parse_python_dependencies import (
ParsePythonDependenciesRequest,
from pants.backend.python.dependency_inference.rules import (
PythonImportDependenciesInferenceFieldSet,
)
from pants.backend.python.dependency_inference.rules import rules as core_rules
from pants.backend.python.framework.django import dependency_inference, detect_apps
from pants.backend.python.target_types import PythonSourceField, PythonSourceTarget
from pants.backend.python.target_types import PythonSourceTarget
from pants.backend.python.util_rules import pex
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.core.util_rules import stripped_source_files
from pants.engine.addresses import Address
from pants.engine.rules import QueryRule
from pants.engine.target import InferredDependencies
from pants.testutil.python_interpreter_selection import (
skip_unless_python27_present,
skip_unless_python37_present,
skip_unless_python38_present,
skip_unless_python39_present,
)
from pants.testutil.rule_runner import QueryRule, RuleRunner
from pants.util.strutil import softwrap
from pants.testutil.rule_runner import RuleRunner


@pytest.fixture
def rule_runner() -> RuleRunner:
return RuleRunner(
rule_runner = RuleRunner(
rules=[
*parse_python_dependencies.rules(),
*stripped_source_files.rules(),
*pex.rules(),
*dependency_inference.rules(),
*detect_apps.rules(),
QueryRule(ParsedPythonDependencies, [ParsePythonDependenciesRequest]),
*core_rules(),
QueryRule(InferredDependencies, [dependency_inference.InferDjangoDependencies]),
],
target_types=[PythonSourceTarget],
)
rule_runner.set_options([], env_inherit={"PATH", "PYENV_ROOT", "HOME"})
return rule_runner


def assert_deps_parsed(
rule_runner: RuleRunner,
content: str,
*,
constraints: str,
expected_imports: dict[str, ImpInfo] | None = None,
expected_assets: list[str] | None = None,
filename: str = "path/to/app0/migrations/0001_initial.py",
) -> None:
expected_imports = expected_imports or {}
expected_assets = expected_assets or []
rule_runner.set_options(
[
"--no-python-infer-string-imports",
"--no-python-infer-assets",
],
env_inherit={"PATH", "PYENV_ROOT", "HOME"},
)
def do_test_migration_dependencies(rule_runner: RuleRunner, constraints: str) -> None:
rule_runner.write_files(
{
"BUILD": f"python_source(name='t', source={repr(filename)})",
"path/to/app1/BUILD": softwrap(
"BUILD": "python_source(name='t', source='path/to/app0/migrations/0001_initial.py')",
"path/to/app0/migrations/0001_initial.py": dedent(
"""\
class Migration(migrations.Migration):
dependencies = [
("app1", "0012_some_migration"),
("app2_label", "0042_another_migration"),
]
operations = []
"""
),
"path/to/app1/BUILD": dedent(
f"""\
python_source(
source="apps.py",
interpreter_constraints=['{constraints}'],
)
"""
),
"path/to/app1/apps.py": softwrap(
"path/to/app1/apps.py": dedent(
"""\
class App1AppConfig(AppConfig):
name = "path.to.app1"
label = "app1"
"""
),
"another/path/app2/BUILD": softwrap(
"path/to/app1/migrations/BUILD": "python_source(source='0012_some_migration.py')",
"path/to/app1/migrations/0012_some_migration.py": "",
"another/path/app2/BUILD": dedent(
f"""\
python_source(
source="apps.py",
interpreter_constraints=['{constraints}'],
)
"""
),
"another/path/app2/apps.py": softwrap(
"another/path/app2/apps.py": dedent(
"""\
class App2AppConfig(AppConfig):
name = "another.path.app2"
label = "app2_label"
"""
),
filename: content,
"another/path/app2/migrations/BUILD": "python_source(source='0042_another_migration.py')",
"another/path/app2/migrations/0042_another_migration.py": "",
}
)
tgt = rule_runner.get_target(Address("", target_name="t"))
result = rule_runner.request(
ParsedPythonDependencies,
InferredDependencies,
[
ParsePythonDependenciesRequest(
tgt[PythonSourceField],
InterpreterConstraints([constraints]),
dependency_inference.InferDjangoDependencies(
PythonImportDependenciesInferenceFieldSet.create(tgt)
)
],
)
assert dict(result.imports) == expected_imports
assert list(result.assets) == sorted(expected_assets)


def do_test_migration_dependencies(rule_runner: RuleRunner, constraints: str) -> None:
content = dedent(
"""\
class Migration(migrations.Migration):
dependencies = [
("app1", "0012_some_migration"),
("app2_label", "0042_another_migration"),
]
operations = []
"""
)
assert_deps_parsed(
rule_runner,
content,
constraints=constraints,
expected_imports={
"path.to.app1.migrations.0012_some_migration": ImpInfo(lineno=3, weak=True),
"another.path.app2.migrations.0042_another_migration": ImpInfo(lineno=4, weak=True),
},
)
assert set(result.include) == {
Address("another/path/app2/migrations", target_name="migrations"),
Address("path/to/app1/migrations", target_name="migrations"),
}


@skip_unless_python27_present
Expand Down
15 changes: 8 additions & 7 deletions src/python/pants/backend/python/framework/django/detect_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@
from collections import defaultdict
from dataclasses import dataclass

from pants.backend.python.dependency_inference.parse_python_dependencies import get_scripts_digest
from pants.backend.python.subsystems.setup import PythonSetup
from pants.backend.python.target_types import InterpreterConstraintsField
from pants.backend.python.util_rules.interpreter_constraints import InterpreterConstraints
from pants.backend.python.util_rules.pex_environment import PythonExecutable
from pants.base.deprecated import warn_or_error
from pants.base.specs import FileGlobSpec, RawSpecs
from pants.engine.fs import AddPrefix, Digest, MergeDigests
from pants.engine.fs import AddPrefix, CreateDigest, Digest, FileContent, MergeDigests
from pants.engine.internals.selectors import Get, MultiGet
from pants.engine.process import Process, ProcessResult
from pants.engine.rules import collect_rules, rule
Expand All @@ -25,6 +24,7 @@
Targets,
)
from pants.util.frozendict import FrozenDict
from pants.util.resources import read_resource


@dataclass(frozen=True)
Expand All @@ -45,8 +45,8 @@ def add_from_json(self, json_bytes: bytes) -> "DjangoApps":
apps = dict(self.label_to_name, **json.loads(json_bytes.decode()))
return DjangoApps(FrozenDict(sorted(apps.items())))

def to_json(self) -> bytes:
return json.dumps(dict(self.label_to_name), sort_keys=True).encode()

_script_resource = "scripts/app_detector.py"


@rule
Expand Down Expand Up @@ -86,9 +86,10 @@ async def detect_django_apps(python_setup: PythonSetup) -> DjangoApps:
if not targets:
return django_apps

script_digest = await get_scripts_digest(
"pants.backend.python.framework.django.scripts", ["app_detector.py"]
script_file_content = FileContent(
"script/__visitor.py", read_resource(__name__, _script_resource)
)
script_digest = await Get(Digest, CreateDigest([script_file_content]))
apps_sandbox_prefix = "_apps_to_detect"

# Partition by ICs, so we can run the detector on the appropriate interpreter.
Expand Down Expand Up @@ -122,7 +123,7 @@ async def detect_django_apps(python_setup: PythonSetup) -> DjangoApps:
Process(
argv=[
python_interpreter.path,
"pants/backend/python/framework/django/scripts/app_detector.py",
script_file_content.path,
apps_sandbox_prefix,
],
input_digest=input_digest,
Expand Down
Loading

0 comments on commit f9188ab

Please sign in to comment.