Add new "python_versions" attribute to the pack metadata file #4474

Kami · 2018-12-14T14:32:21Z

This pull request adds new python_version attribute to the pack metadata file.

This attribute uses semver notation to specify which Python versions pack works with (e.g. >= 2.7.0, <= 3.7.0).

The goal is to utilize this attribute for various things:

On pack installation, check if to be installed pack is compatible with Python version under which StackStorm is running
Advertise Python 2.x / Python 3.x compatibility on pack exchange
Utilize this attribute in the pack CI - only runs tests and checks for Python version pack is compatible with and consider failures as fatal

Once we agree on the attribute "design", I / we will go and manually populate this field for various most commonly used pack based on manual and automated testing (StackStorm-Exchange/ci#61).

Related PR: StackStorm-Exchange/ci#61

Open Questions

I went with semver approach because we already use that for stackstorm_version attribute (consistency).

After more thought, I think it's too complicated - we could make it simpler by only advertising general python 2 and python 3 compatibility by adding {"python_versions": "python2": true, "python3": false} or similar attribute to pack metadata.

This attribute is different from stackstorm_version one - in StackStorm version case, you might want to specify different minor and patch versions and this should rarely be the case with Python version.

What do others think?

complex_semver_match function.

action.

…n_pack_metadata

pack.yaml file for future compatibility reasons.

jeking3 · 2018-12-17T14:00:08Z

Use semver - it's standard.

lakshmi-kannan · 2018-12-17T17:01:28Z

Although semver is standard and which is what we use for st2 version, the python version really doesn't need the level of details. We are really interested in the python major versions. So something like "python_major_versions": [2, 3] is an option too.

Kami · 2019-01-08T09:28:35Z

@jeking3 Thanks for your input, but after more thinking about it, I agree with @lakshmi-kannan here.

semver is too complicated in this specific scenario. We only care about the major version (so Python 2.x or Python 3.x) and complex semver expression would just confuse end users and pack developers.

Kami · 2019-01-08T10:35:46Z

Another thing worth noting - packs will only work with with Python 2.x and Python 3.x versions supported by the platform (aka StackStorm packages and components).

This means it makes no sense for pack to also specify Python version beyond the major one (aka also specifying minor and patch version).

Minor and patch versions will always be dictated by the platform.

Right now that is 2.7.x and 3.6.x. That means that packs which declare support for Python 2 are tested and work with Python 2.7.x and same goes for Python 3.

…st2 into python_version_pack_metadata

make it a list. This attribute specifies which major Python versions the pack works with (2, 3). If this attribute is not specified in pack.yaml, we perform no Python version compatibility check when installing and registering the pack. The check can also be skipped by using --force install flag, but that is at user's own risk and packs are not guaranteed to work. Update affected tests.

Kami · 2019-01-08T14:20:00Z

I've pushed necessary changes which add new python_versions attribute which is a list (as discussed above).

Valid values for that field are "2", "3: and combination of those two. I made it a string, but if other think it makes more sense, I can also make it an integer (I didn't make it an integer because we will never do number like comparisons on it).

jeking3

Just a few small comments.

jeking3 · 2019-01-08T15:34:42Z

st2common/st2common/models/api/pack.py

@@ -100,6 +100,22 @@ class PackAPI(BaseAPI):
                               '">=1.8.0, <2.2.0"',
                'pattern': ST2_VERSION_REGEX,
            },
+            'python_versions': {
+                'type': 'array',
+                'description': ('Major Python versiosn supported by this pack. E.g. '


Typo "versiosn".

Thanks, will fix it.

jeking3 · 2019-01-08T15:35:11Z

st2common/st2common/models/api/pack.py

+            'python_versions': {
+                'type': 'array',
+                'description': ('Major Python versiosn supported by this pack. E.g. '
+                                '"2" for Python 2.7.x and "3" for Python 3.6.x'),


This will change over time, is it wise to put the minor versions in here? I could see this getting stale.

As mentioned here (#4474 (comment)), the minor and patch version itself are dictated by the StackStorm platform and the packages we ship.

We could also store it here, but I don't think it would serve any good purpose - the information itself would be duplicated and provide no additional value, just potential confusion.

The idea is if you are using official StackStorm packages under Python 2 and pack metadata says it works under Python 2, it should work. Same goes for Python 3.

My point was that the string on line 106 will become incorrect over time.

jeking3 · 2019-01-08T15:37:00Z

st2common/st2common/models/api/pack.py

+                'maxItems': 2,
+                'uniqueItems': True,
+                'additionalItems': True
+            },


What is the default for packs that do not specify it?

We don't specify required=True which means it's an optional attribute. If it's not provided, we don't perform any checks during pack install and register phase (we just assume it works with the version under which StackStorm is currently running).

That's done for backward compatibility reasons.

Why maxItems here? This means we have to update this code whenever a new major version of Python is released. Which, granted, doesn't happen all that often, but if packs want to specify a bunch of different values, I don't see how that is a problem.

jeking3 · 2019-01-08T15:37:21Z

st2common/st2common/models/api/pack.py

+                        '3'
+                    ]
+                },
+                'minItems': 1,


Without a default and minItems = 1 will this break all existing packs?

Nope - min items only applies to this attribute when it's provided. If the attribute itself is not provided, minItems validation check is not performed.

We also have various test cases which cover that scenarios (we have a lot of pack test fixtures without that attribute).

Kami · 2019-01-08T16:15:22Z

@jeking3 Thanks for taking a look and your feedback.

I pushed a fix for the typo and I believe I addressed all of your comments. If I missed something, please let me know.

blag · 2019-01-08T19:47:15Z

st2common/st2common/models/api/pack.py

+                'maxItems': 2,
+                'uniqueItems': True,
+                'additionalItems': True
+            },


Why maxItems here? This means we have to update this code whenever a new major version of Python is released. Which, granted, doesn't happen all that often, but if packs want to specify a bunch of different values, I don't see how that is a problem.

jeking3

My issues were resolved and questions answered to satisfaction.

Kami · 2019-01-09T08:40:52Z

@blag Re maxItems - that's correct, but it doesn't happen often.

And if a new version of Python is released this means we will likely need to update our code and definitely update our packages since we bundle the virtualenv with Python interpreter inside the packages.

In short - if a new version for Python is released and some user wants to use it, we will need to update our code (packs CI, etc.) anyway and at that time we will update maxItems as well.

arm4b · 2019-03-26T16:08:06Z

@Kami Is this feature documented somewhere?
Couldn't find any references in st2docs.

Kami · 2019-03-26T16:54:51Z

@armab Not yet - feel free to create an issue and assign it for me. I will go to it in time for v3.0.0.

arm4b · 2019-03-26T17:39:09Z

contrib/packs/tests/test_action_download.py


        result = action.run(packs=['test3'], abs_repo_base=self.repo_base)
        self.assertEqual(result['test3'], 'Success.')

        # Pack requires a version which is not satisfied by current StackStorm version
-        st2common.util.pack_management.CURRENT_STACKSTROM_VERSION = '2.2.0'
+        st2common.util.pack_management.CURRENT_STACKSTORM_VERSION = '2.2.0'


There is definitely a 100 and 1 way to misspell StormStack 😃

arm4b · 2019-03-26T17:41:59Z

@Kami Sounds good, here we go StackStorm/st2docs#870

Kami added 6 commits December 3, 2018 23:05

Add new python_version attribute to the pack model.

9eda5bf

Fix typo.

02f1767

Add support for special "all" version specifier to the

688a842

complex_semver_match function.

Add support for installation Python version check to the download

8ed7ab1

action.

Fix typo.

0487e4e

Add a test case for it.

123cc33

Kami added this to the 3.0.0 milestone Dec 14, 2018

Kami added 3 commits December 17, 2018 10:01

Merge branch 'master' of github.com:StackStorm/st2 into python_versio…

400ca62

…n_pack_metadata

Add explicit additionalProperties: True to PackAPI model.

7e3cf7f

Add a test case which verifies we allow arbitrary properties inside

474710c

pack.yaml file for future compatibility reasons.

Kami mentioned this pull request Jan 4, 2019

Continuous Integration for StackStorm Exchange packs using Python 3 #4490

Closed

Merge branch 'master' of github.com:StackStorm/st2

5346908

Kami mentioned this pull request Jan 8, 2019

Add support for Python 3.x checks to pack Circle CI config StackStorm-Exchange/ci#61

Merged

3 tasks

Merge branch 'master' into python_version_pack_metadata

385b3ca

Kami changed the title ~~[WIP] Add new "python_version" attribute to the pack metadata file~~ Add new "python_versions" attribute to the pack metadata file Jan 8, 2019

Kami added 3 commits January 8, 2019 11:40

Merge branch 'master' of github.com:StackStorm/st2

d81bca1

Merge branch 'python_version_pack_metadata' of github.com:StackStorm/…

3b23f5b

…st2 into python_version_pack_metadata

Merge branch 'master' into python_version_pack_metadata

37bdd67

Kami added the python3 label Jan 8, 2019

Kami added 4 commits January 8, 2019 14:13

Add some additional tests.

85c15f5

Fix formatting.

a7c7861

Add tests for pack registrar and python_version attribute.

f245f17

Add missing fixture file.

01765f6

jeking3 reviewed Jan 8, 2019

View reviewed changes

Fix typo.

2731d5a

blag approved these changes Jan 8, 2019

View reviewed changes

jeking3 reviewed Jan 8, 2019

View reviewed changes

Kami merged commit 5872d6e into master Jan 9, 2019

Kami deleted the python_version_pack_metadata branch January 9, 2019 08:45

Kami added a commit that referenced this pull request Jan 9, 2019

Add changelog entry for #4474.

c8286f0

Kami mentioned this pull request Jan 9, 2019

Python versions changelog entry #4494

Merged

LindsayHill mentioned this pull request Jan 24, 2019

Feature Request: Specify Python version in Pack Metadata #4386

Closed

arm4b reviewed Mar 26, 2019

View reviewed changes

arm4b mentioned this pull request Mar 26, 2019

Document new 'python_versions' in st2 pack metadata StackStorm/st2docs#870

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new "python_versions" attribute to the pack metadata file #4474

Add new "python_versions" attribute to the pack metadata file #4474

Kami commented Dec 14, 2018

jeking3 commented Dec 17, 2018

lakshmi-kannan commented Dec 17, 2018

Kami commented Jan 8, 2019 •

edited

Loading

Kami commented Jan 8, 2019

Kami commented Jan 8, 2019

jeking3 left a comment

jeking3 Jan 8, 2019

Kami Jan 8, 2019

jeking3 Jan 8, 2019

Kami Jan 8, 2019

jeking3 Jan 8, 2019

jeking3 Jan 8, 2019

Kami Jan 8, 2019

blag Jan 8, 2019

jeking3 Jan 8, 2019

Kami Jan 8, 2019 •

edited

Loading

Kami commented Jan 8, 2019

blag Jan 8, 2019

jeking3 left a comment

Kami commented Jan 9, 2019

arm4b commented Mar 26, 2019

Kami commented Mar 26, 2019

arm4b Mar 26, 2019

arm4b commented Mar 26, 2019

Add new "python_versions" attribute to the pack metadata file #4474

Add new "python_versions" attribute to the pack metadata file #4474

Conversation

Kami commented Dec 14, 2018

Open Questions

jeking3 commented Dec 17, 2018

lakshmi-kannan commented Dec 17, 2018

Kami commented Jan 8, 2019 • edited Loading

Kami commented Jan 8, 2019

Kami commented Jan 8, 2019

jeking3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kami Jan 8, 2019 • edited Loading

Choose a reason for hiding this comment

Kami commented Jan 8, 2019

Choose a reason for hiding this comment

jeking3 left a comment

Choose a reason for hiding this comment

Kami commented Jan 9, 2019

arm4b commented Mar 26, 2019

Kami commented Mar 26, 2019

Choose a reason for hiding this comment

arm4b commented Mar 26, 2019

Kami commented Jan 8, 2019 •

edited

Loading

Kami Jan 8, 2019 •

edited

Loading