https://pantsbuild.org/ logo
#development
Title
# development
p

proud-dentist-22844

02/10/2023, 12:46 AM
I just finished upgrading my plugins to support 2.15 (from 2.14). 😰 the Fmt/lint plugin API changes were a bear this time because I had to not only change the rules and add some new rules, I also had to add new subsystems in each of my plugins where I didn't need a subsystem before. Good changes, just more churn with less tooling to help me do the upgrade than in previous releases.
b

bitter-ability-32190

02/10/2023, 2:13 AM
Technically you should've had a subsystem. It's a bit of a harsh viewpoint, but part of my motivation for these declarative changes is pushing conformity. You could (and maybe should) argue plugin authors maybe shouldn't need the same conformity that core pants plugins do. But I also think to the end users it's all "pants". They don't see the difference.
That being said. I feel your pain. I make the breaking change, and then 2 months later I curse myself as I do my plugins. It's one of a few reasons I try and upstream everything
p

proud-dentist-22844

02/10/2023, 2:15 AM
Technically you should've had a subsystem.
Ah, but I used
PexFromTargetsRequest
to run the "linter" and "formatter" defined within the repo. So, there was no subsystem before as I did not need to define any external tool details.
I ended up using the subsystem to consolidate some of the constants defined in my plugins, so it is in a sense a bit cleaner. But much less straight forward.
b

bitter-ability-32190

02/10/2023, 2:19 AM
The subsystem requirement is more of a forcing function for your
--skip
option
p

proud-dentist-22844

02/10/2023, 2:20 AM
As far as upstreaming goes, If we could have an abstract
PexFromTargets
that can act as a formatter and a linter, then I could delete 3 of my plugins which are just light wrappers to integrate in-repo scripts into pants
fmt
and
lint
goals.
b

bitter-ability-32190

02/10/2023, 2:21 AM
There's an issue that would get us there
p

proud-dentist-22844

02/10/2023, 2:21 AM
The adhoc_tool stuff?
b

bitter-ability-32190

02/10/2023, 2:21 AM
No, it'd be shifting the tools from their own lockfile into being specified as a target if your own resolve.
(if the user wants)
p

proud-dentist-22844

02/10/2023, 2:22 AM
Fascinating. I don't think I've seen that issue/discussion. Got a link?
b

bitter-ability-32190

02/10/2023, 2:23 AM
p

proud-dentist-22844

02/10/2023, 2:53 AM
pants-plugins lines changed in the upgrade: [+354 -187] https://github.com/StackStorm/st2/compare/pants-upgrade-2.15 In general, I don't care about LOC... But, it makes a huge difference in getting people to review changes. The smaller the LOC, the faster I'll get a review.
b

bitter-ability-32190

02/10/2023, 2:55 AM
Hmm unless you have custom partitioners you should theoretically be having fewer LOC.
p

proud-dentist-22844

02/10/2023, 2:56 AM
https://github.com/pantsbuild/pants/issues/12449
This has to do with moving the version definition for existing backends from pants.toml into BUILD files. I don't think it would help with eliminating my fmt/lint plugins by taking a random target and saying that its output should be used as a fmt or a lint tool.
b

bitter-ability-32190

02/10/2023, 2:57 AM
If that target is a
python_requirement
that's exactly what that ticket describes
p

proud-dentist-22844

02/10/2023, 2:58 AM
It's not, it's an in-repo python_sources / entry point
b

bitter-ability-32190

02/10/2023, 2:58 AM
...honestly that might even work too
There no PR. So nothing to say it wouldn't work 😅
p

proud-dentist-22844

02/10/2023, 2:58 AM
True.
b

bitter-ability-32190

02/10/2023, 2:59 AM
FWIW we now have adhoc tests through shell commands. I filed a ticket to do the same for fmt and lint
That might also be a route?
p

proud-dentist-22844

02/10/2023, 2:59 AM
My "custom partitioner" just passes field_sets to my fmt rules instead of the files list. Very short.
Copy code
@rule()
async def generate_schemas_partitioner(
    request: GenerateSchemasViaFmtTargetsRequest.PartitionRequest,
) -> Partitions[GenerateSchemasFieldSet, _EmptyMetadata]:
    return Partitions.single_partition(request.field_sets)
b

bitter-ability-32190

02/10/2023, 2:59 AM
Technically a formatter/linter should be anything `run`nable in a sandbox
There should be a enum value that matches that, is there not?
p

proud-dentist-22844

02/10/2023, 3:01 AM
Not quite. I was only able to use
partitioner_type = PartitionerType.DEFAULT_SINGLE_PARTITION
for 1 lint rule and 1 fmt rule. The other 2 fmt rules needed field_sets instead of files.
b

bitter-ability-32190

02/10/2023, 3:02 AM
You need field sets, do you subclass the Target formatter subclass?
p

proud-dentist-22844

02/10/2023, 3:02 AM
yeah
But the assumption for fmt/fix is that they will eventually act on files.
🤔 1
And I needed to pull in some transitive dependencies for this fmt rule that is actually codegen in fmt clothing.
b

bitter-ability-32190

02/10/2023, 3:03 AM
Ah ok
Well then I don't feel as bad 😅
p

proud-dentist-22844

02/10/2023, 3:04 AM
lol
Please don't take my feedback as saying your hard work was bad... It's just my experience so far.
b

bitter-ability-32190

02/10/2023, 3:04 AM
Would love to make your case simpler, but not in the hot path, so to speak
Oh no, I stand by my work. And the pain it causes. Eventually it will converge............ I hope
😂
😆 1
p

proud-dentist-22844

02/10/2023, 7:58 AM
Blast. That assumption that fix/fmt deals with files (and therefore batch elements is a list of strings) bit me. Something in rust is trying to cast my FieldSet to PyString, and it gets stuck in an endless loop. My plugin tests pass, but my test rule requests must be bypassing the logic in the fmt goal that triggers that. 😞 So: https://github.com/StackStorm/st2/actions/runs/4141002684/jobs/7160185247 `[ERROR] panic at 'called
Result::unwrap()
on an
Err
value: "Field
globs
was not convertible to type alloc:vecVec&lt;allocstringString&gt; PyErr { type: <class 'TypeError'>, value: TypeError(\"'APISpecFieldSet' object cannot be converted to 'PyString'\"), traceback: None }"', src/nodes.rs:770` Oddly enough, I don't get the rust ERROR locally, I only get the `Filesystem changed during run: retrying
Lint
in 500ms...` message in an endless loop. I guess I'm not done updating my plugins for 2.15
I think it might be the
fix_batch
rule.
b

bitter-ability-32190

02/10/2023, 12:44 PM
The core code is opaque to partition element types. Put another way, whatever your partition rule returns gets passed into your runner rule. I'm that way, it doesn't have to operate in field sets or files
ohhhhhhhh right I remember. Lint is opque. Fmt/Fix can't be. Will reply in other thread
4 Views