[VPEX][1/8] Add local-env foundation: result types and env-key mapping#5823
Open
rugpanov wants to merge 5 commits into
Open
[VPEX][1/8] Add local-env foundation: result types and env-key mapping#5823rugpanov wants to merge 5 commits into
rugpanov wants to merge 5 commits into
Conversation
Contributor
Waiting for approvalCould not determine reviewers from git history. Eligible reviewers: Suggestions based on git history. See OWNERS for ownership rules. |
Collaborator
Integration test reportCommit: be61b7d
19 interesting tests: 15 SKIP, 4 RECOVERED
Top 5 slowest tests (at least 2 minutes):
|
rugpanov
force-pushed
the
dbconnect/01-engine
branch
from
87c371d to
865a2cd
Compare
rugpanov
changed the title
This was referenced Jul 3, 2026
First of a stacked series adding `databricks local-env python sync`, which provisions a local Python environment matched to a Databricks compute target. The feature lands across small, single-concern PRs; each layer is independently reviewable and adds no user-facing surface until the final PR wires the command in. This PR is the foundation the rest of the stack builds on: - result.go: the result types and the --json / E_* error contract that every phase reports through (Result, PipelineError, ErrorCode, PhaseName, PhaseStatus, Mode, TargetInfo, ResolvedInfo, Plan, Warning), plus the command-path constants (local-env / python / sync) defined once. - envkey.go: mapping a compute target to an environment key and parsing the Python minor from a requires-python specifier. Nothing imports this package yet, so the CLI is unchanged. The unexported filesystem/artifact constants and the canonical phase-order slice live with the pipeline that consumes them (a later PR in the stack). Co-authored-by: Isaac
rugpanov
force-pushed
the
dbconnect/01-engine
branch
from
865a2cd to
22f99d9
Compare
rugpanov
changed the title
…pecifiers Review of the foundation layer flagged that PythonMinorFromRequires took the first MAJOR.MINOR in the string via first-match regex. For a multi-clause requires-python where the exclusive upper bound comes first — e.g. "<3.13,>=3.10" — it returned 3.13, the version the "<3.13" clause forbids, because PEP 440 clause order is arbitrary. The result feeds PM.EnsurePython, so the tool could target a Python the constraint excludes. Prefer a lower-bound / pinning clause (>=, >, ==, ~=, ===) and only fall back to the first version when none is present. Adds multi-clause test coverage; the prior tests exercised only single-bound specifiers. Co-authored-by: Isaac
…dden version Round-2 review of the foundation layer noted that when a requires-python has no lower-bound/pin clause at all (only upper-bound or exclusion, e.g. "<3.13,!=3.12"), PythonMinorFromRequires fell back to the first number and returned 3.13 — a version the specifier forbids. Such a spec has no floor to install from, so it now errors rather than guessing. A bare "3.12" (no operator) is still accepted as a valid floor. Co-authored-by: Isaac
…ower bound
Round-3 review found two edge cases in the regex-based PythonMinorFromRequires:
with multiple lower bounds (">=3.8,>=3.11") it returned the first (3.8) rather
than the effective floor (3.11), and a bare floor alongside an exclusion
("!=3.11,3.12") was wrongly rejected as having no floor.
Replaced the layered regexes with a small clause parser: split on commas,
classify each clause by operator (>=,>,==,~=,=== and bare = floor; <,<=,!=
never a floor), and return the highest floor. A spec with no floor clause
("<3.13", "!=3.12") still errors. Covers multi-lower-bound, bare-floor +
exclusion, ordering, and whitespace.
Co-authored-by: Isaac
Round-4 review noted PythonMinorFromRequires returned 3.10 for ">3.10", but PEP 440's ">" excludes the entire given release series (neither 3.10 nor any 3.10.x satisfies ">3.10"), so the lowest installable minor is 3.11. The clause parser now bumps the minor by one for a strict ">" bound. Co-authored-by: Isaac
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
This is PR 1 of 8 in a stacked series adding the
databricks local-env python synccommand, which provisions a local Python environment (Python version,databricks-connectpin, and dependency constraints) matched to a selected Databricks compute target. An earlier revision carried the whole engine (~2.3k lines) in one diff — too large to review — so it has been decomposed into small, single-concern PRs:target.go)constraints.go)merge.go)Hidden: true)The feature is not reachable by users until the final PR, so nothing here changes CLI behavior.
What this PR contains
The foundation the rest of the stack builds on:
result.go— the result types and the--json/E_*error contract that every phase reports through:Result,PipelineError,ErrorCode,PhaseName,PhaseStatus,Mode,TargetInfo,ResolvedInfo,Plan,Warning. It also defines the command-path constants (local-env/python/sync) in one place.envkey.go— mapping a compute target to an environment key (EnvKeyForServerless,EnvKeyForSparkVersion,NormalizeServerless) and parsing the Python minor from arequires-pythonspecifier.Dormant by construction
Nothing imports this package (
libs/localenv) yet, so the CLI is unchanged. The unexported filesystem/artifact constants and the canonical phase-order slice live with the pipeline that consumes them (PR 5), keeping this layer to just the contract types — which is also what keeps itunused- anddeadcode-clean on its own.This pull request and its description were written by Isaac.