Skip to content

pants: register assets (like conf files) and dependencies on them #5846

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 26 commits into from
Dec 30, 2022

Conversation

cognifloyd
Copy link
Member

Background

This is another part of introducing pants, as discussed in various TSC meetings.

Related PRs can be found in:

Overview of this PR

So far, we have told pants about our python and our shell sources. ./pants tailor :: made that much easier. But we have more than just *.py and *.sh files; we have conf, ini, yaml, json, and many other kinds of files. In pants, these miscellaneous files are all "assets", and tailor does not know what to do with them. So, we need to register the assets manually.

This PR registers most of our assets with one of these targets: file, files, resource, or resources.

This PR also registers dependencies on these assets. I think I captured all the dependencies, but we may have to tweak them if I missed something.

I recommend stepping through the commits so that you can quickly see what depends on the assets added in that commit.

Relevant Pants documentation

Files vs Resources

It can be confusing to figure out whether we need to register the assets as files or resources. In my PoC branch, I started with files and then gradually switched things over to resources when needed (like for building wheels). We will probably have to tweak file/resource on some of these in the future.

aside: There have been some discussions in the pantsbuild community about how file and resource could be combined into an asset target. Whether or not that happens, registering assets in pants will hopefully improve with time. For now, just think of "assets" as a helpful term to refer to both "files" and "resources".

Future PRs

  • Follow-up PRs will introduce some plugins that add specific handling to some of the resources/files in this PR. This way, pants will be able to auto-generate or lint things like the api spec. I will make those once this PR is merged.

  • I will register the LICENSE file in conjunction with the metadata that we need to build wheels.

  • There is also a large set of assets that I have intentionally skipped in this PR: pack metadata (including yaml files for packs, actions, workflows, rules, etc, and a few other misc files that get included in packs). I wrote a pants plugin to handle registering these assets because there are a lot of them. The plugin teaches ./pants tailor :: how to handle those files, automatically adding a custom pack_metadata target.

@cognifloyd cognifloyd added this to the pants milestone Dec 12, 2022
@cognifloyd cognifloyd self-assigned this Dec 12, 2022
@pull-request-size pull-request-size bot added the size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review. label Dec 12, 2022
@cognifloyd cognifloyd force-pushed the pants-files-resources branch from 5b35e9c to c365b7c Compare December 12, 2022 03:13
Comment on lines +6 to +9
file(
name="st2.conf.sample",
source="st2.conf.sample",
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A future PR will add pants-plugins/sample_conf which will wire this up so that it gets regenerated whenever someone runs the fmt goal with something like: ./pants fmt conf/st2.conf.sample. That will take advantage of the fine-grained caching so that this always gets updated when the files used to generate it change.

Comment on lines +8 to +11
resources(
name="openapi_spec",
sources=["openapi.yaml", "openapi.yaml.j2"],
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A future PR will add pants-plugins/api_spec which will wire this up so that it gets regenerated whenever someone runs the fmt goal with something like: ./pants fmt st2common/st2comon/openapi.yaml. That will take advantage of the fine-grained caching so that openapi.yaml always gets updated when the files used to generate it change.

Comment on lines +1 to +4
resources(
name="policies_meta",
sources=["*.yaml"],
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding this BUILD file broke a test that was checking directory contents. So, I adjusted the test to account for it.

cognifloyd added a commit that referenced this pull request Dec 18, 2022
this will not be needed once #5846 is merged
Comment on lines +1 to +10
python_sources(
dependencies=[
":jsonschema",
]
)

resources(
name="jsonschema",
sources=["*.json"],
)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is required before I can submit the pants plugin that wires up pants to manage building the schemas in contrib/schemas.

@cognifloyd cognifloyd force-pushed the pants-files-resources branch 2 times, most recently from dbda1f2 to 12f0dde Compare December 19, 2022 20:31
@cognifloyd cognifloyd requested review from amanda11 and a team December 19, 2022 20:43
@cognifloyd cognifloyd disabled auto-merge December 29, 2022 05:24
@cognifloyd cognifloyd enabled auto-merge December 29, 2022 05:24
@cognifloyd cognifloyd force-pushed the pants-files-resources branch 4 times, most recently from c33a175 to 341f372 Compare December 29, 2022 15:17
cognifloyd added a commit that referenced this pull request Dec 29, 2022
this will not be needed once #5846 is merged
Let pants know where to get these conf files.
Note that these conf files are not the same as the files
in the repo root: //conf/st2.tests*.conf
No technical reason. Just trying to be more consistent.
'artifacts' refers more to distributable stuff like wheels.
'assets' encompasses 'file' and 'resource' targets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pantsbuild size/XL PR that changes 500-999 lines. Consider splitting work into several ones that easier to review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants