forked from pantsbuild/pants
-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
whitespaace #3
Open
mateor
wants to merge
1
commit into
foursquare:1.4.0+fsX
Choose a base branch
from
mateor:mateo.1.4.0+fsX-fork
base: 1.4.0+fsX
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
whitespaace #3
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
jonshea
pushed a commit
that referenced
this pull request
Nov 11, 2020
Any time that you add a new top-level goal in V2—that is a "verb" for Pants like `test`, `fmt`, `setup-py`, and `dependencies`—you must define the same three basic things: ``` Goal, GoalSubsystem, @console_rule ``` One of those does not fit with the other two. In contrast, these three all belong: ``` Goal, GoalSubsystem, @goal_rule ``` ### What is an `@console_rule`? There are (at least) 3 ways of conceptualizing what an `@console_rule` means: 1. A rule that has exclusive access to the `Console`. * Only `@console_rules` can write to stdout and stderr. * This property is represented in the name `@console_rule`. 2. A rule that is not cached. * This is an important property of `@console_rules` and contrasts with normal `@rules`. * Per the conversation in pantsbuild#8931 (comment), this idea on uncachability is the main mental model for some. 3. A rule that maps 1-1 with a top-level goal / returns an `engine.goal.Goal`. All three of these properties are true and important components of an `@console_rule`. However, I argue that #3 is the most important: _fundamentally, an `@console_rule` is a special rule that hooks up to a Pants goal_. Right now, the name `@console_rule` optimizes for property #1. In contrast, `@goal_rule` focuses on property #3. (NB: all three of these properties will continue applying regardless of the name) ### Leveraging prior Pants knowledge Most Pants users are familiar with the idea of a goal. It's the [very first concept introduced in our docs](https://www.pantsbuild.org/first_concepts.html) and every single Pants run involves invoking goals, regardless of V1 vs. V2. In contrast, fewer users are familiar with the `Console` abstraction, which is never mentioned in our docs and is more of an implementation detail for how V2 Pants works. We do expect users to soon write their own rules in plugins. The name `@goal_rule` will allow them to leverage their prior knowledge, just as `subsystem_rule` leverages prior knowledge better than `optionable_rule` did.
jonshea
pushed a commit
that referenced
this pull request
Nov 11, 2020
…uild#9466) Twice now, we've had `Sources` fields that require a specific number of files. So, this PR formalizes a mechanism to do this. Error message #1: > ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 3 files, but it had 1 file. Error message #2: > ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have 2 or 3 files, but it had 1 file. Error message #3: > ERROR: The 'sources' field in target build-support/bin:check_banned_imports must have a number of files in the range `range(20, 20000, 10)`, but it had 1 file. [ci skip-rust-tests] # No Rust changes made. [ci skip-jvm-tests] # No JVM changes made.
jonshea
pushed a commit
that referenced
this pull request
Nov 11, 2020
## Goals of design See https://docs.google.com/document/d/1tJ1SL3URSXUWlrN-GJ1fA1M4jm8zqcaodBWghBWrRWM/edit?ts=5ea310fd for more info. tl;dr: 1) Protocols now only have one generic target, like `avro_library`. This means that call sites must declare which language should be generated from that protocol. * Must be declarative. 2) You can still get the original protocol sources, e.g. for `./pants filedeps`. 3) Must work with subclassing of fields. 4) Must be extensible. * Example: Pants only implements Thrift -> Python. A plugin author should be able to add Thrift -> Java. ## Implementation Normally, to hydrate sources, we call `await Get[HydratedSources](HydrateSourcesRequest(my_sources_field))`. We always use the exact same rule to do this because all `sources` fields are hydrated identically. Here, each codegen rule is unique. So, we need to use unions. This means that we also need a uniform product for each codegen rule for the union to work properly. This leads to: ```python await Get[GeneratedSources](GenerateSourcesRequest, GeneratePythonFromAvroRequest(..)) await Get[GeneratedSources](GenerateSourcesRequest, GenerateJavaFromThriftRequest(..)) ``` Each `GenerateSourcesRequest` subclass gets registered as a union rule. This achieves goal #4 of extensibility. -- To still work with subclassing of fields (goal #3), each `GenerateSourcesRequest` declares the input type and output type, which then allows us to use `isinstance()` to accommodate subclasses: ```python class GenerateFortranFromAvroRequest(GenerateSourcesRequest): input = AvroSources output = FortranSources ``` -- To achieve goals #1 and #2 of allowing call sites to declaratively either get the original protocol sources or generated sources, we hook up codegen to the `hydrate_sources` rule and `HydrateSourcesRequest` type: ```python protocol_sources = await Get[HydratedSources](HydrateSourcesRequest(avro_sources, for_sources_types=[FortranSources], codegen_enabled=True)) ``` [ci skip-rust-tests] [ci skip-jvm-tests]
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.
Problem
(explain the context of the problem and why you're making this change. include
references to all relevant github issues.)
Solution
(describe the modifications you've made.)
Result
(describe how your changes affect the end-user behavior of the system. this section is
optional, and should generally be summarized in the title of the pull request.)