st2 icon indicating copy to clipboard operation
st2 copied to clipboard

Pants plugins: pack_metadata - python imports rules

Open cognifloyd opened this issue 1 year ago • 0 comments

This PR has commits extracted from #6202 where I'm working on getting all of our unit tests to run under pants+pytest.

It is best to review each commit of this PR to make the review manageable (I rewrote history to simplify reviews). If this PR is still too large to review, I can try to split it into multiple PRs.

Overview

This has pants+pytest run pack tests directly instead of using st2-run-pack-tests, which has some issues detailed below.

This enhances pants-plugins/pack_metadata (introduced in #5868) so that it can inject the custom PYTHONPATH entries that StackStorm adds when running python in packs like actions and sensors. We have several special lib directories to support:

  • <pack>/lib is the shared lib dir that gets added for both actions and sensors when [packs].enable_common_libs = True in st2.conf.
  • <pack>/actions/lib for actions (allowing import foo to import <pack>/actions/lib/foo.py in an action)
  • The entry_point parent directory for sensors and actions

That last point is actually not handled correctly in st2-run-pack-tests which only adds <pack>/actions/ and <pack>/sensors/ to PYTHONPATH, which assumes that the python file is directly in those directories. The python files can, however be in a sub directory, and that sub directory gets added to PYTHONPATH. So, this pants-plugins/pack_metadata enhancement allows pants to accurately reflect the logic st2 actually uses.

Another reason that this does not use st2-run-pack-tests, is that it obscures the PYTHONPATH logic that pants needs to carefully manage. Pants uses PEX to make sure that tests are run in a hermetic sandbox that cannot be influenced by things like packages installed with pip install --user. So, the python dependencies detected by this plugin actually get passed as PEX_EXTRA_SYS_PATH not PYTHONPATH which is ignored when running python hermetically.

Eventually, I would like to publish pants-plugins/pack_metadata so it can be used in pack repos to run tests with pants+pytest instead of st2-run-pack-tests.

Plugin components

Developing pants plugins

Pants has extensive documentation on the plugin API, including how to create targets, and how to write rules. Some of the plugin APIs I'm using in this PR are not documented, but these still give the general overview of the rule+target mechanisms used.

Targets and Target Fields

pack_metadata and pack_content_resource Targets and their Fields

The pack_metadata target was introduced in #5868, and it was added to BUILD files in #5871. For example, this is the pack_metadata target for the linux pack:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/contrib/linux/BUILD#L3-L5

Creating this doesn't require manual intervention. If a new pack is added, CI will prompt you to run pants tailor :: which generates this target (see #5871). One of my goals with this PR is to preserve this low-maintenance aspect while introducing the features we need.

So, what does the pack_metadata target do? It is basically a resources target that generates a resource target for each of the yaml files in the pack: pack.yaml, action metadata, sensor metadata, rules, etc. This is similar to the python_sources target which generates a python_source target for each python file.

In order to calculate the PYTHONPATH entries for python actions and sensors, the plugin has to extract the entry_point from the individual metadata files. But, the pack_metadata target generated generic resource targets that didn't make it easy to distinguish action metadata from pack.yaml and other files. So, this PR changes pack_metadata so it generates a pack_content_resource target for each file instead of just using the generic resource target. Note that the pack_content_resource target should not actually show up in any BUILD files. It should only be generated by pack_metadata.

This is where the pack_metadata target says which target should be generated for each file:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/target_types.py#L167

This PR adds the pack_content_resource target here:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/target_types.py#L148-L156

It has the same fields as resource target with a couple additions. It extends ResourceTarget so that anything that needs a generic resource can still get the file. Part of that is extending the ResourceSourceField because targets are often identified by the their fields not by the target itself.

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/target_types.py#L111-L112

The other new field is type, defined here. The possible values are defined in PackContentResourceTypes which is an enum. In nearly all cases, the BUILD files will not have an explicit type for any of these targets, so the compute_value method will get raw_value=None. It then uses a map to calculate the actual value for type based on the file's path (in address).

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/target_types.py#L71-L84

If a pack has files that do not follow conventions, this PR adds an escape hatch for overriding the calculated type. The pack_metadata target now has the ResourcesOverridesField to provide this escape hatch. It would be used like this:

pack_metadata(
    name="metadata",
    overrides={
        "actions/workflows/some-action-chain.yaml": dict(
            type="action_chain_workflow",  # default is orquesta_workflow
        ),
    },
)

The conventions for types of metadata files in subdirectories are defined here:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/target_types.py#L59-L68

If a file doesn't follow any of the conventions, it gets type="unknown", which can be overridden with the overrides field (shown above).

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/target_types.py#L108

None of the packs in the st2 repo break with convention, except for maybe some insignificant things in fixture packs. So, I didn't have to add any overrides to get pack and unit tests passing.

New Field: python_tests(inject_pack_python_path=...)

This PR also has to provide a way for python_tests targets to opt-in to the PYTHONPATH manipulation. For example, the linux pack's BUILD file uses __defaults__ to enable this for all tests in the pack:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/contrib/linux/BUILD#L1

I added this by hand, instead of making pants tailor :: add it, because it's not needed for fixture packs. So, I added it only for the main packs that might actually get installed.

This new field is added to python_tests and python_test targets here:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/register.py#L37-L40

And the field is defined as a simple Boolean field here:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/target_types.py#L207-L215

Rules

This PR adds the actual PYTHONPATH manipulation logic in 3 sets of rules, with each set in a different file:

  • pack_metadata/python_rules/python_pack_content.py
  • pack_metadata/python_rules/python_module_mapper.py
  • pack_metadata/python_rules/python_path_rules.py

Note: This PR also adds tests for each of these rules.

I added a long comment that provides an overview of the implementation and how these rules work together:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/python_rules/python_pack_content.py#L51-L86

python_pack_content

The rules in this file are responsible for finding pack content using StackStorm's conventions. Any rules that need to extract something pack metadata files should be in this file (like the rule that extracts entry_point from action and sensor metadata files).

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/python_rules/python_pack_content.py#L98-L104

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/python_rules/python_pack_content.py#L178-L181

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/python_rules/python_pack_content.py#L294-L299

python_module_mapper

This file has a rule to inject StackStorm's custom python module lookup logic into the pants dependency inference, since the standard python import rules cannot identify pack python files. The dependency inference logic influences which files are considered dependencies of other files, and therefore which files need to be in the sandbox when running pytest and other tools on that file.

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/python_rules/python_module_mapper.py#L42-L48

python_path_rules

This file has rules to calculate StackStorm's custom pack PYTHONPATH and then inject that (as PEX_EXTRA_SYS_PATH) when pants runs pytest.

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/python_rules/python_path_rules.py#L48-L54

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/pants-plugins/pack_metadata/python_rules/python_path_rules.py#L111-L117

Eventually, I hope to add this PYTHONPATH manipulation for pylint as well (and possibly other utils python runs), but that will require changing pants itself to support that. Since this only affected one file (a file we don't even lint in the soon-to-be-legacy Makefile), I just skipped the pylint error:

https://github.com/StackStorm/st2/blob/b3d8c4c0dd442257fd12622538cb1cb45bed09eb/contrib/examples/actions/pythonactions/isprime.py#L18-L19

cognifloyd avatar Oct 05 '24 18:10 cognifloyd