[VPEX][2/8] Add local-env compute-target resolution#5824
Open
rugpanov wants to merge 2 commits into
Open
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: 436dabf
21 interesting tests: 14 SKIP, 7 RECOVERED
Top 5 slowest tests (at least 2 minutes):
|
rugpanov
force-pushed
the
dbconnect/01-engine
branch
from
865a2cd to
22f99d9
Compare
rugpanov
force-pushed
the
dbconnect/02-target
branch
from
5fdb8b6 to
2092138
Compare
rugpanov
changed the title
rugpanov
force-pushed
the
dbconnect/02-target
branch
from
2092138 to
22d1137
Compare
rugpanov
force-pushed
the
dbconnect/02-target
branch
from
8d309ae to
d9b9c50
Compare
rugpanov
force-pushed
the
dbconnect/02-target
branch
from
d9b9c50 to
77ac2c8
Compare
Second in the stacked local-env series (builds on the foundation types). target.go resolves a compute target to a TargetInfo (and its environment key) using ordered precedence: --cluster flag → --serverless flag → --job flag → bundle target. Compute lookups go through the narrow ComputeClient seam so the resolver is unit-tested against a stub with no SDK dependency. ValidateTargetFlags guards the library path against more than one target flag being set. The classic-compute job branch reads the Spark version from the first return of GetJobSparkVersion, per that method's documented contract, rather than the recorded-version third return. Depends on the foundation PR for NewError, the E_RESOLVE / E_NO_TARGET codes, TargetInfo, and the EnvKeyFor* helpers. Still dormant. Co-authored-by: Isaac
Review of the target layer noted that ResolveTarget accepted incompatible
flags on the library path: called directly with e.g.
TargetFlags{Cluster: "c", Serverless: "v4"} it silently took the first
precedence branch and ignored the rest, resolving a different target than
requested. Cobra and the cmd layer already reject this, but ResolveTarget is
exported and ValidateTargetFlags exists specifically to guard callers that
bypass Cobra, so the resolver now runs that check first and returns
E_RESOLVE on conflicting flags.
Co-authored-by: Isaac
rugpanov
force-pushed
the
dbconnect/02-target
branch
from
77ac2c8 to
436dabf
Compare
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
PR 2 of 8 in the stacked
databricks local-env python syncseries (see #5823 for the full plan). Stacked on #5823 — review that first; this PR's diff is onlytarget.go+ its test.What this PR contains
target.go— compute-target resolution. Resolves a target to aTargetInfoand its environment key using ordered precedence:ComputeClientseam, so the resolver is unit-tested against a stub with no SDK dependency.ValidateTargetFlagsguards the library path against more than one target flag being set (Cobra enforces this at the CLI layer too, in a later PR).GetJobSparkVersion, per that method's documented contract, not the recorded-version third return.Dependencies & surface
Depends on the foundation PR (#5823) for
NewError, theE_RESOLVE/E_NO_TARGETcodes,TargetInfo, and theEnvKeyFor*helpers. Still dormant — nothing imports the package yet.This pull request and its description were written by Isaac.