Mask the environment name in graph and specs_rules
#17179
Merged
Conversation
This file contains 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
# Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
… a `@rule`'s identity. [ci skip-rust] [ci skip-build-wheels]
# Building wheels and fs_util will be skipped. Delete if not intended. [ci skip-build-wheels]
[ci skip-rust] [ci skip-build-wheels]
[ci skip-build-wheels] [ci skip-rust]
…ly adding an `EnvironmentName` to all product requests and test `QueryRule`s. [ci skip-rust] [ci skip-build-wheels]
[ci skip-build-wheels]
stuhood
added
the
category:internal
|
Commits are useful to review independently! |
chrisjrn
approved these changes
Oct 10, 2022
Comment on lines
+89
to
+90
| #[test] | ||
| fn masked_params() { |
There was a problem hiding this comment.
It'd be good to test when masking works
| @@ -304,6 +304,31 @@ fn in_scope_computed() { | |||
| graph.find_root_edges(vec![], "a").unwrap(); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn in_scope_provided() { | |||
There was a problem hiding this comment.
Should this be cherry-picked?
There was a problem hiding this comment.
in_scope_types is only on main so far.
Eric-Arellano
approved these changes
Oct 10, 2022
Some of it, yea. We'll almost certainly expose other things that we want masked as part of #17155. |
stuhood
added a commit
that referenced
this pull request
Oct 11, 2022
As described in #17129: we would like `@goal_rule`s to eventually make their own decisions about which environments to use, generally by consuming targets to do so, but possibly also by pinning themselves to other environments via configuration. This change builds on #17179, and introduces a migration from `Goal.environment_migrated = {False => True}`. All graph-introspection goals consume only the APIs which are pinned to the local environment by #17179, and so are trivially migrated here. Other goals trigger a deprecation warning like the following: ``` DEPRECATED: Not setting `Goal.environment_migrated=True` for `Goal` `generate-lockfiles` is scheduled to be removed in version 2.17.0.dev0. See https://www.pantsbuild.org/v2.15/docs/plugin-upgrade-guide ``` Before calling #17129 done, we will migrate all internal goals to `Goal.environment_migrated=True`. [ci skip-rust] [ci skip-build-wheels]
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
As a first step toward #17129, this change "masks" the
EnvironmentNamefor relevant APIs ingraph.pyandspecs_rules.py, and pins them to the__local__environment.The
@rule(.., _mask_types=[..])argument triggers an assertion if the given type(s) are used in the identity of the@rule(i.e.: if the@rule"consumes" them from its scope). The impact of masking a type at an API boundary is an assertion that that type is not consumed by that API.As described in #17129: we're pinning graph computations to the
__local__environment for now, although we may eventually allow graph computations to execute in a configurable environment.[ci skip-build-wheels]