-
-
Notifications
You must be signed in to change notification settings - Fork 775
Add new "python_versions" attribute to the pack metadata file #4474
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
Changes from all commits
9eda5bf
02f1767
688a842
8ed7ab1
0487e4e
123cc33
400ca62
7e3cf7f
474710c
5346908
385b3ca
d81bca1
3b23f5b
37bdd67
015ef43
85c15f5
a7c7861
f245f17
01765f6
2731d5a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -100,6 +100,22 @@ class PackAPI(BaseAPI): | |
| '">=1.8.0, <2.2.0"', | ||
| 'pattern': ST2_VERSION_REGEX, | ||
| }, | ||
| 'python_versions': { | ||
| 'type': 'array', | ||
| 'description': ('Major Python versions supported by this pack. E.g. ' | ||
| '"2" for Python 2.7.x and "3" for Python 3.6.x'), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will change over time, is it wise to put the minor versions in here? I could see this getting stale.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My point was that the string on line 106 will become incorrect over time. |
||
| 'items': { | ||
| 'type': 'string', | ||
| 'enum': [ | ||
| '2', | ||
| '3' | ||
| ] | ||
| }, | ||
| 'minItems': 1, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Without a default and minItems = 1 will this break all existing packs?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope - min items only applies to this attribute when it's provided. If the attribute itself is not provided, We also have various test cases which cover that scenarios (we have a lot of pack test fixtures without that attribute). |
||
| 'maxItems': 2, | ||
| 'uniqueItems': True, | ||
| 'additionalItems': True | ||
| }, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the default for packs that do not specify it?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't specify That's done for backward compatibility reasons.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why |
||
| 'author': { | ||
| 'type': 'string', | ||
| 'description': 'Pack author or authors.', | ||
|
|
@@ -144,7 +160,10 @@ class PackAPI(BaseAPI): | |
| 'description': 'Location of the pack on disk in st2 system.', | ||
| 'required': False | ||
| } | ||
| } | ||
| }, | ||
| # NOTE: We add this here explicitly so we can gracefuly add new attributs to pack.yaml | ||
| # without breaking existing installations | ||
| 'additionalProperties': True | ||
| } | ||
|
|
||
| def __init__(self, **values): | ||
|
|
@@ -189,6 +208,7 @@ def to_model(cls, pack): | |
| version = str(pack.version) | ||
|
|
||
| stackstorm_version = getattr(pack, 'stackstorm_version', None) | ||
| python_versions = getattr(pack, 'python_versions', []) | ||
| author = pack.author | ||
| email = pack.email | ||
| contributors = getattr(pack, 'contributors', []) | ||
|
|
@@ -200,7 +220,8 @@ def to_model(cls, pack): | |
| model = cls.model(ref=ref, name=name, description=description, keywords=keywords, | ||
| version=version, author=author, email=email, contributors=contributors, | ||
| files=files, dependencies=dependencies, system=system, | ||
| stackstorm_version=stackstorm_version, path=pack_dir) | ||
| stackstorm_version=stackstorm_version, path=pack_dir, | ||
| python_versions=python_versions) | ||
| return model | ||
|
|
||
|
|
||
|
|
||
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.
There is definitely a 100 and 1 way to misspell StormStack 😃