-
-
Notifications
You must be signed in to change notification settings - Fork 747
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
Pants: Add pack_metadata
targets to BUILD files
#5871
Conversation
Several fixtures have been adjusted so pants can infer them. So, I dropped the hard-coded dep in the BUILD files.
This reverts commit 4a1b241.
pack_metadata
targets to BUILD files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A request for clarification...
@@ -1 +1,7 @@ | |||
python_sources() | |||
pack_metadata( | |||
name="metadata", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly confused. In the dummy_pack_23 we don't include the pack_metadata as there is no pack.yaml in that pack.
For things like this BUILD file and others at this level in the hierarchy, why do they have the pack_metatdata?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pack_metadata
includes more than just pack.yaml
.
st2/pants-plugins/pack_metadata/target_types.py
Lines 27 to 40 in a73225c
class PackMetadataSourcesField(ResourcesGeneratingSourcesField): | |
required = False | |
default = ( | |
# metadata does not include any python, shell, or other sources. | |
"pack.yaml", | |
"config.schema.yaml", | |
"*.yaml.example", | |
"**/*.yaml", | |
"**/*.yml", | |
"icon.png", # used in st2web ui | |
# "requirements*.txt", # including this causes target conflicts | |
# "README.md", | |
# "HISTORY.md", | |
) |
It also includes metadata yaml files for resources that go in packs: actions, aliases, sensors, etc.
Several of the directories at this level do not have pack.yaml
, but they do have the other metadata yaml files. Examples:
- st2tests/st2tests/fixtures/aliases/
- st2tests/st2tests/fixtures/backstop/
- rules/
- triggers/ this is not common in packs, but is allowed.
- triggertypes/ this is even less common, and I'm not sure if it can be defined in packs, but close enough.
- st2tests/st2tests/fixtures/descendants/ this is not like a pack, but has db entries in the form of yaml files which is sort of pack-ish
- executions/ this would never be defined in a pack
- child1_level1
- ...
- executions/ this would never be defined in a pack
Then the other fixtures do similar things. They all seem to have mongo docs in the form of yaml files (after-all the db effectively just holds all of the metadata files in json format, so going in the reverse makes sense).
We need resources
for all of these yaml files. PackMetadata
subclasses Resources
, so everything in the pack_metadata
target also counts as a resource. So, we either need to:
- add
resources(sources=...)
targets that encompass all of the yaml files, and make the python fixture.py depend on them - add
pack_metadata
and make the python fixture.py depend on it
It was quick and easy to just treat these as packs using pack_metadata
so that's what I did. Would it make more sense to you if we used resources()
instead for these?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh. And one more clarification, dummy_pack_23
does not have ANY yaml files, not just pack.yaml. So, including the pack_metadata target there results in warnings because none of the sources globs match anything. All of these other fixtures do have yaml files, so the **/*.yaml
glob matches.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's fine - thanks for clarification.
My point for not reading the resource correctly / I did go and look and must have not scrolled past the pack example yaml line.
All good!
pack_metadata( | ||
name="metadata", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would be functionally equivalent without using the pack_metadata
target. Would you prefer this for these fixtures?
pack_metadata( | |
name="metadata", | |
) | |
resources( | |
name="metadata", | |
sources=["**/*.yaml"], | |
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Background
This is another part of introducing pants, as discussed in various TSC meetings.
Related PRs can be found in:
Overview of this PR
This PR adds
pack_metadata
targets, introduced in #5868 to our BUILD files.If someone runs
./pants tailor ::
, it will automatically addpack_metadata
targets in any directory with apack.yaml
file, so when we add new fixtures, it should not be much work to keep the metadata up-to-date.There are several fixture packs that don't have a
pack.yaml
file, so I added those targets manually. Also, for the fixtures, I made the python fixture depend on the pack metadata, so that pants will include all of a pack's metadata files if a test depends on that fixture.I also added one
pack_metadata_in_git_submodule
target, which I described in #5868 as:Here is that target (sources have to be explicitly defined because they target is defined in a parent directory, not alongside the
pack.yaml
file):st2/st2tests/st2tests/fixtures/packs/BUILD
Lines 6 to 14 in 26132d1
And here is what that error message looks like in practice:
And then once you checkout the git submodule, your command will succeed. For example, here is running tailor successfully (this also shows the list of
pack.yaml
files that tailor could detect automatically):