Allow plugin authors to attach custom fields to TargetData for pants peek

[ndellosa95]

This PR adds the ability to attach custom additional_info to the output of pants peek as described in the feature request here.

[kaos]

Love this idea.

Would prefer to add one additional level per union member to further scope the additional data to avoid potential conflicts from different backends.

[ndellosa95]

Love this idea.

Would prefer to add one additional level per union member to further scope the additional data to avoid potential conflicts from different backends.

What're you thinking? I was going back and forth between whether the label attribute should be on the HasAdditionalTargetDataFieldSet or the AdditionalTargetData instances - maybe just do both?

Also, can you add the appropriate category label to the PR, so the check here passes. I don't think I have permissions to do so.

[kaos]

Love this idea.
Would prefer to add one additional level per union member to further scope the additional data to avoid potential conflicts from different backends.

What're you thinking? I was going back and forth between whether the label attribute should be on the HasAdditionalTargetDataFieldSet or the AdditionalTargetData instances - maybe just do both?

They're 1-to-1, right? So I just realized that adding a extra level gives little, what I want is to ensure the key used is unique, and the data returned can be a (frozen) dict if structured data is to be set.

Or we trust people to use a sensibly unique name for the labels.. 🤷🏽 ?
Should have proper docs/examples that highlight the recommended naming for them though (perhaps along the lines of reverse domain names a la java and k8s annotations etc..)

I think between the two, having it on the AdditionalTargetData makes more sense, to keep it flexible based on which target it returns data for (can have a single union member for multiple targets that way).

Also, can you add the appropriate category label to the PR, so the check here passes. I don't think I have permissions to do so.

Done.

[ndellosa95]

Love this idea.
Would prefer to add one additional level per union member to further scope the additional data to avoid potential conflicts from different backends.

What're you thinking? I was going back and forth between whether the label attribute should be on the HasAdditionalTargetDataFieldSet or the AdditionalTargetData instances - maybe just do both?

They're 1-to-1, right? So I just realized that adding a extra level gives little, what I want is to ensure the key used is unique, and the data returned can be a (frozen) dict if structured data is to be set.

Or we trust people to use a sensibly unique name for the labels.. 🤷🏽 ? Should have proper docs/examples that highlight the recommended naming for them though (perhaps along the lines of reverse domain names a la java and k8s annotations etc..)

I think between the two, having it on the AdditionalTargetData makes more sense, to keep it flexible based on which target it returns data for (can have a single union member for multiple targets that way).

Also, can you add the appropriate category label to the PR, so the check here passes. I don't think I have permissions to do so.

Done.

I mentioned it as another way we could categorize, i.e. {"additional_info": {"my_plugin_target_type": {"my_additional_info": <etc>}}} but it also doesn't really get us much that just trusting people to name their labels sensibly doesn't.

Should I make edits to the docs to highlight the inclusion of this feature?

[kaos]

Love this idea.
Would prefer to add one additional level per union member to further scope the additional data to avoid potential conflicts from different backends.

What're you thinking? I was going back and forth between whether the label attribute should be on the HasAdditionalTargetDataFieldSet or the AdditionalTargetData instances - maybe just do both?

They're 1-to-1, right? So I just realized that adding a extra level gives little, what I want is to ensure the key used is unique, and the data returned can be a (frozen) dict if structured data is to be set.
Or we trust people to use a sensibly unique name for the labels.. 🤷🏽 ? Should have proper docs/examples that highlight the recommended naming for them though (perhaps along the lines of reverse domain names a la java and k8s annotations etc..)
I think between the two, having it on the AdditionalTargetData makes more sense, to keep it flexible based on which target it returns data for (can have a single union member for multiple targets that way).

Also, can you add the appropriate category label to the PR, so the check here passes. I don't think I have permissions to do so.

Done.

I mentioned it as another way we could categorize, i.e. {"additional_info": {"my_plugin_target_type": {"my_additional_info": <etc>}}} but it also doesn't really get us much that just trusting people to name their labels sensibly doesn't.

Agreed. Let's keep it simple for now.

Should I make edits to the docs to highlight the inclusion of this feature?

Would be nice, if you find a sensible spot for it, perhaps somewhere under "Writing Plugins"?

[ndellosa95]

Love this idea.
Would prefer to add one additional level per union member to further scope the additional data to avoid potential conflicts from different backends.

What're you thinking? I was going back and forth between whether the label attribute should be on the HasAdditionalTargetDataFieldSet or the AdditionalTargetData instances - maybe just do both?

They're 1-to-1, right? So I just realized that adding a extra level gives little, what I want is to ensure the key used is unique, and the data returned can be a (frozen) dict if structured data is to be set.
Or we trust people to use a sensibly unique name for the labels.. 🤷🏽 ? Should have proper docs/examples that highlight the recommended naming for them though (perhaps along the lines of reverse domain names a la java and k8s annotations etc..)
I think between the two, having it on the AdditionalTargetData makes more sense, to keep it flexible based on which target it returns data for (can have a single union member for multiple targets that way).

Also, can you add the appropriate category label to the PR, so the check here passes. I don't think I have permissions to do so.

Done.

I mentioned it as another way we could categorize, i.e. {"additional_info": {"my_plugin_target_type": {"my_additional_info": <etc>}}} but it also doesn't really get us much that just trusting people to name their labels sensibly doesn't.

Agreed. Let's keep it simple for now.

Should I make edits to the docs to highlight the inclusion of this feature?

Would be nice, if you find a sensible spot for it, perhaps somewhere under "Writing Plugins"?

Added below the synthetic targets section in Concepts.

[kaos]

Very nice 👍🏽

[ndellosa95]

Very nice 👍🏽

Thanks! I don't have permissions to merge so whenever you get the chance, it would be greatly appreciated.

[kaos]

Very nice 👍🏽

Thanks! I don't have permissions to merge so whenever you get the chance, it would be greatly appreciated.

yep, wanted to give a window for others to comment.. but I see this PR has been open for quite some time, so I'll merge and if there are any late comments we can always address them in a follow up PR if needed.

[kaos]

Related: #18546