[internal] Refactor jvm_artifact usage
#13890
Conversation
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
| # Materialise the existing lockfile, and check for changes. We don't want to re-write | ||
| # identical lockfiles |
There was a problem hiding this comment.
We were duplicating the two if statement bodies. I inverted the conditional to make this all shorter and simpler.
| raise CoursierError( | ||
| raise AssertionError( |
There was a problem hiding this comment.
This represents a programmer error.
| group = target[JvmArtifactGroupField].value | ||
| if not group: | ||
| raise InvalidTargetException( | ||
| f"The `group` field of {target.alias} target {target.address} must be set." | ||
| ) | ||
|
|
||
| artifact = target[JvmArtifactArtifactField].value | ||
| if not artifact: | ||
| raise InvalidTargetException( | ||
| f"The `artifact` field of {target.alias} target {target.address} must be set." | ||
| ) | ||
|
|
||
| version = target[JvmArtifactVersionField].value | ||
| if not version: | ||
| raise InvalidTargetException( | ||
| f"The `version` field of {target.alias} target {target.address} must be set." | ||
| ) |
There was a problem hiding this comment.
The Target API already was checking that it's not None. We just needed to teach MyPy by setting value: str, as MyPy doesn't understand that required = True means the value is no longer Optional.
| # ----------------------------------------------------------------------------------------------- | ||
| # Generic resolve support fields | ||
| # ----------------------------------------------------------------------------------------------- |
There was a problem hiding this comment.
This section is moved, unmodified, from below. I'll need it in a followup PR defined here so that I can subclass it lower in the file.
# Rust tests and lints will be skipped. Delete if not intended. [ci skip-rust] # Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
| if url and jar: | ||
| raise CoursierError( | ||
| "You cannot specify both a `url` and `jar` for the same `jvm_artifact`." | ||
| ) |
There was a problem hiding this comment.
Now done with the Target API via Target.validate(). The major benefit of that is the validation is eager, ./pants list :: will check it. We generally have preferred eager errors.
The downside is that this is less reslient to creating a new JvmArtifact-like target in a plugin, they might not have implemented Target.validate too.
[ci skip-rust]
[ci skip-build-wheels]