Makes check a USES_ENVIRONMENTS goal
#17315
Conversation
(cherry picked from commit 15f3f36)
src/python/pants/core/goals/check.py
Outdated
| for _ in request.field_sets | ||
| ) | ||
| grouped = itertools.groupby( | ||
| zip(request_for_each_environment_name, environment_names), lambda x: x[0] |
There was a problem hiding this comment.
Rather than zipping, consider emitting both the request and environment name in request_for_each_environment_name?
There was a problem hiding this comment.
A zip is required somewhere because it's not possible to get both the EnvironmentName and the requests into the same object. That said, I've tidied up the relevant code so it's a bit clearer what's going on. New commit incoming.
chrisjrn
added
the
category:internal
| environment_names = await MultiGet( | ||
| Get( | ||
| EnvironmentName, | ||
| EnvironmentNameRequest, | ||
| EnvironmentNameRequest.from_field_set(field_set), | ||
| ) | ||
| for (_, field_set) in request_to_field_set | ||
| ) | ||
|
|
||
| request_to_env_name = { | ||
| (request, env_name) | ||
| for (request, _), env_name in zip(request_to_field_set, environment_names) | ||
| } |
There was a problem hiding this comment.
If a request has multiple field sets, this might end up computing multiple environment names for one request, which will overwrite one another in this dict.
This code probably needs to either partition by environment name or error for that case? See #17328 on that topic: currently mypy internally partitions, but check itself does not.
There was a problem hiding this comment.
This is a literal for a set of (request, env_name) tuples, not a dict, so it'll run each request in each environment for which it is valid.
There was a problem hiding this comment.
I was considering whether this rule should actually re-construct each request with only the field sets that match each EnvironmentName, but for now, this is probably fine?
There was a problem hiding this comment.
I was considering whether this rule should actually re-construct each request with only the field sets that match each
EnvironmentName, but for now, this is probably fine?
Yea, that would effectively be partitioning, á la #17328. I expect that that will be necessary in the medium term.
checkwill now run eachCheckRequestin each specified environment per thefield_setsin theCheckRequest. It looks like the remainder of thecheckinfrastructure already successfully handles dealing with multipleCheckRequests.See #17129