Implement preemptive skipping in publish goal
#23052
Conversation
benjyw
left a comment
benjyw
left a comment
There was a problem hiding this comment.
What's the use-case for publish without package? How important is it? I ask because this is a little convoluted, and I'm wondering if it's more straightforward to implement a skip flag/field for packaging, similar to what we have for publishing, and then not attempt to publish whatever wasn't packaged.
Or am I way off base?
The code looks fine, although a little confusing, I will dive a little deeper, but wanted to get that question out there.
|
|
||
|
|
||
| @dataclass(frozen=True) | ||
| class CheckSkipResult: |
There was a problem hiding this comment.
Whoops, didn't intend to delete your comment here...
There was a problem hiding this comment.
What's the use-case for publish without package?
It's a specific workaround for Docker images where --output type=registry or --output type=image,push=true which will immediately push the image to a remote registry when built. This breaks Pants existing model of package then publish.
My thinking was we could skip the packaging for these and then run the actual build during the publish step. Then you could set the [docker].push_on_package option from my other PR to error or ignore and prevent pants package from pushing your image to a remote registry.
There was a problem hiding this comment.
Ah yeah, so doing the skipping on packaging wouldn't really work. So the approach in this PR, cumbersome as it is, is probably the way to go.
| If `inner` is None, this indicates that this request should NOT be skipped. | ||
| """ | ||
|
|
||
| inner: tuple[PublishPackages, ...] |
There was a problem hiding this comment.
Can we name this something more evocative?
There was a problem hiding this comment.
Like "packages_to_skip" or whatever?
Anyway, once this is renamed I will approve and merge.
2 participants
Closes #23029
This adds a new polymorphic rule to run skip checks for the publish goal preemptively, i.e. before packaging, which is precursor to closing this issue. This implementation also supports only skipping packaging but still running the publish rule for a target, which we can use to implement push packaging for
docker_imagetargets (another precursor to #23030).Also had Claude whip up some docs, lightly edited by me, on adding a publisher in the plugins docsite.