Closed
Conversation
cosmicexplorer
force-pushed
the
query-pipelining
branch
from
1d39cfb to
d6118c1
Compare
7 tasks
stuhood
reviewed
Apr 5, 2019
| diffspec=None, | ||
| include_dependees=changes_since.include_dependees, | ||
| # TODO: is this what we want? see the --changed-fast option! | ||
| # TODO(#7346): parse --query expressions using the python ast module to support keyword args! |
| hydrate_noop, | ||
| noop_results, | ||
| # ???/intersection | ||
| RootRule(IntersectionOperands), |
Member
There was a problem hiding this comment.
Is this a root for testing purposes? If so, could add the root in the test instead...
| """ | ||
| :param Options options: An `Options` instance to use. | ||
| :param session: The Scheduler session | ||
| # TODO: `symbol_table` is unused!! |
| else: | ||
| specs = literal_specs | ||
|
|
||
| # TODO: this somehow deadlocks if you yield Specs instead of InitialSpecs -- FIX THIS!!! |
Member
There was a problem hiding this comment.
Hm... deadlocks or livelocks? What does the rule graph look like?
|
|
||
|
|
||
| @rule(ScmResult, [Select(ScmRequest)]) | ||
| def try_get_scm(scm_request): |
Member
There was a problem hiding this comment.
I think that leaving the wrapped SCM as a parameter is a bit cleaner, for the reason mentioned on #7350.
| the --query option, the testing for that functionality should be moved into this file (or into | ||
| unit testing), and the separate option or goal should eventually be deprecated. | ||
| """ | ||
| # TODO(#7346): test changes-since or changes-for-diffspec queries! It's currently difficult to set |
Member
There was a problem hiding this comment.
Can take a look at the harness in https://github.com/pantsbuild/pants/blob/master/tests/python/pants_test/engine/legacy/test_changed_integration.py for an example.
cosmicexplorer
force-pushed
the
query-pipelining
branch
4 times, most recently
from
ba6e9e6 to
52b8fd7
Compare
cosmicexplorer
force-pushed
the
query-pipelining
branch
2 times, most recently
from
51fe6ed to
030bb1b
Compare
cosmicexplorer
force-pushed
the
query-pipelining
branch
7 times, most recently
from
f033af2 to
04fe733
Compare
cosmicexplorer
force-pushed
the
query-pipelining
branch
2 times, most recently
from
8da6be8 to
955df30
Compare
cosmicexplorer
force-pushed
the
query-pipelining
branch
2 times, most recently
from
e1fea47 to
1d0085d
Compare
use the ast module to parse query expressions!! # Rust tests will be skipped. Delete if not intended. [ci skip-rust-tests]
cosmicexplorer
force-pushed
the
query-pipelining
branch
2 times, most recently
from
2d50ea1 to
df4212b
Compare
(by making sure that the transitive hydrated targets are all within the previous range of addresses for filters) turn the Changed subsystem into v2 rules for target root calculation fix --owner-of add --query option to cover owner-of, changes-since, etc at once make an integration test and make it pass clean up convert QueryComponentMixin -> just QueryComponent actually add integration test! tag the query integration test target as such, and use text_type() move query.py up to src/python/pants/engine because it's not legacy make getting changed files uncacheable! fix test to avoid trailing newlines ...add query.py move --query integration testing out of engine/legacy/ make things work after rebase (the test passes now!) apply UnionRule to QueryComponent! get to the point where we start wanting to return union values add QueryPipeline! fixes after rebase (broken) remove legacy query save point IT WORKS!!! ok, pipelining works again add new query operators...unreally easily show specific object on hash error return just the initial specs actually make query pipelining work (by making sure that the transitive hydrated targets are all within the previous range of addresses for filters) make target selection conjunctions work!! fix query integration test registration add minimize() query operation!! fixes after rebase [ci skip-jvm-tests] # No JVM changes made. [ci skip-rust-tests]
cosmicexplorer
force-pushed
the
query-pipelining
branch
from
df4212b to
a46d2fe
Compare
Contributor
|
Closing as stale, which we're doing for all changes that haven't been touched in 1+ years to simplify project management. |
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
3 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.
WIP until #7350 is merged. The diff against #7350 is at https://github.com/cosmicexplorer/pants/compare/query-option-for-unifying-target-selection...cosmicexplorer:query-pipelining?expand=1.
Problem
It would be neat to unify our separate target selection mechanisms (e.g.
--owner-of,filter) under a single interface. This discussion began in #6501. See #7350 for the first diff in the series described by #7346.We can make
--querya list option and process each element as a filter, similar tojq(described in #6501 (comment)). This PR implements that.Solution
QueryPipelineand theprocess_query_pipelinerule to processBuildFileAddressesafter applying each query expression as a filter using the output of the previous expression as input.--owner-of,--changed-*, or literal target specs, just as we currently do.owner-of,changes-since,changes-for-diffspec, andnoop), and not enough testing at all.Result
--queryexpressions can now be pipelined for more complex queries.