bitter-ability-32190
09/06/2022, 3:50 PMlint
then batches the partitions and hands those batches with their metadata back to pylint
for running
• Therefore the metadata must be valid to each batch in a partition, because the metadata isn't batched
So we need to make the partition metadata be performantly valid for all possible batches of the partition's FieldSets.
So, that'd the context behind the change, in case that informs your review thoughts. Eventually the pylint runner gets a bag of field_sets and static metadata applying to that-bag-or-biggerwitty-crayon-22786
09/06/2022, 4:37 PMlint
would want to partition CoarsenedTargets
then, probably.bitter-ability-32190
09/06/2022, 4:37 PMwitty-crayon-22786
09/06/2022, 4:38 PMpylint
and mypy
both need transitive deps, rather than just one level of deps, for examplebitter-ability-32190
09/06/2022, 4:39 PMwitty-crayon-22786
09/06/2022, 4:39 PMCan we not partition by ICs/resolve, then when running pylint request CoarsenedTargets on the field set batch?no: as mentioned in my comment, that would trigger #11270: https://github.com/pantsbuild/pants/pull/16761#discussion_r962063128
no: as mentioned in my comment, that would trigger #11270hm. so, i suppose i didn’t mention in there that #11270 applies to both CT and TT. the thing that #11270 is about is avoiding making lots of requests for CTs/TTs (i.e. a request per root or partition), if possible.
bitter-ability-32190
09/06/2022, 4:49 PMwitty-crayon-22786
09/06/2022, 4:50 PMI guess what we could do is attach the CT/TT for a partition with the partition, and in the “runner” construct the sandbox using a possibly-over-populated sandbox for the partition’s CT/TT (even though we’re linting a subset).it’s more that what you partition is CoarsenedTargets, not Targets.
bitter-ability-32190
09/06/2022, 4:53 PMlint.py
into "batches" which are subsets. So either we get the CT/TT for the "batch" in the runner, or we've passed a hint along from the partitioner (which would be maybe over-specified for the batch)witty-crayon-22786
09/06/2022, 4:54 PMCoarsenedTargets
), then you can partition it into one or more lists of CTsdef partition(cts: CoarsenedTargets) -> list[CoarsenedTargets]:
..
bitter-ability-32190
09/06/2022, 4:58 PMlint.py
finds 26 field sets: A..Z
• lint.py
requests partitions from pylint
• pylint
partitions the field sets by some criteria and attaches some metadata.
◦ Let's say its by vowel/consonant
◦ So pylint
returns back 2 partitions: [A,E,I,O,U]+"vowel"
and [<consonant>]+"consonant"
• lint.py
takes that and batches it for cache perf.
◦ let's say batch size is 5
◦ lint.py
gives the pylint
runner 6 _batches_:
▪︎ [A,E,I,O,U]+"vowel"
+ 5 batches of [<consonant subset>]+"consonant"
witty-crayon-22786
09/06/2022, 4:59 PM•requests partitions fromlint.py
pylint
•don’t partition field sets: partition CTspartitions the field sets by some criteria and attaches some metadata.pylint
<http://graph.rs|graph.rs>
thoughbitter-ability-32190
09/06/2022, 5:01 PMdon’t partition field sets: partition CTsIn
pylint
? or in lint
? I think I'm still off one stepping stone 🙂witty-crayon-22786
09/06/2022, 5:03 PMbitter-ability-32190
09/06/2022, 5:06 PMlint.py
should be partitioning by `CT`s, no? Thats wasteful for non-dependent linterswitty-crayon-22786
09/06/2022, 5:07 PMpylint
being in check
would make this easier, since all of those implementations require transitive deps.bitter-ability-32190
09/06/2022, 5:08 PMwitty-crayon-22786
09/06/2022, 5:09 PMTTs
/ CTs
as you can. if you can’t create them at the very beginning of the process, then create them as early as you canbitter-ability-32190
09/06/2022, 5:10 PMwitty-crayon-22786
09/06/2022, 5:11 PMsubset = [ct for ct in cts if any(m in $mybatch for m in ct.members)]
bitter-ability-32190
09/06/2022, 5:12 PMwitty-crayon-22786
09/06/2022, 5:21 PMbitter-ability-32190
09/06/2022, 5:22 PMCoarsenedTargets
for all field sets, then subset that as necessary!witty-crayon-22786
09/06/2022, 8:34 PMbitter-ability-32190
09/06/2022, 8:41 PMlint
code doesn't actually care that the partitions/batches are of field sets after the initial partitioning. The only reason it matters is the dataclass attribute name.
I'm hoping to keep it that way, for sanity 🙂
(The alternative being to split a partition of CTs into batches of CTs)witty-crayon-22786
09/06/2022, 10:33 PMbitter-ability-32190
09/07/2022, 1:28 AMCorsenedTargets
.
Bueno?hundreds-father-404
09/07/2022, 4:03 PMOh hmmmmmmmmm, one issue with this I think is that if any target in the closure of the partition changes, it'll invalidate all batches. As the metadata attached is for the entire partition.that sounds right
I think I'm gonna change the PR to have batches be type-agnostic, then we can batch the CorsenedTargets.vs?
bitter-ability-32190
09/07/2022, 4:05 PMfield_sets
, however it isn't a type-restriction but just a matter of the name of the attribute (it made sense to do it this way and involved less refactors to keep the old name).
Once`lint.py` gets a partition object, it not longer cares about the type past the fact that it can make batches (so an iterable). The batches are then passed back to the "runner" and results are collectedhundreds-father-404
09/07/2022, 4:05 PMbitter-ability-32190
09/07/2022, 4:06 PMRequirementsPexRequest
dopes a transitive walk, as does PythonSourceFilesRequest
witty-crayon-22786
09/09/2022, 5:01 PMbitter-ability-32190
09/09/2022, 6:13 PM