-
Notifications
You must be signed in to change notification settings - Fork 471
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
Update Python requirements for Ansible inclusion requirements document #281
Changes from all commits
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 | ||||
---|---|---|---|---|---|---|
|
@@ -85,34 +85,36 @@ Python requirements for a collection vary between **controller environment** and | |||||
|
||||||
One example scenario where the "even if" clause comes into play is when using cloud modules. These modules mostly run on the controller node but in some environments, the controller might run on one machine inside a demilitarized zone which cannot directly access the cloud machines. The user has to have the cloud modules run on a bastion host/jump server which has access to the cloud machines. | ||||||
|
||||||
An **eligible controller Python version** for a collection is a Python version that is supported on the controller side by at least one ansible-core version that the collection supports. Similarly, an **eligible target Python version** for a collection is a Python version that is supported on the target side by at least one ansible-core version that the collection supports. The eligible controller and target Python versions can be determined from the :ref:`ansible_core_support_matrix` and from the ``requires_ansible`` value in ``meta/runtime.yml`` in the collection. | ||||||
|
||||||
.. _coll_controller_req: | ||||||
|
||||||
Controller environment | ||||||
~~~~~~~~~~~~~~~~~~~~~~ | ||||||
|
||||||
In the controller environment, collections MUST support Python 2 (version 2.7) and Python 3 (Version 3.6 and higher), unless required libraries do not support these versions. Collections SHOULD also support Python v3.5 if all required libraries support this version. | ||||||
Collections MUST support all eligible controller Python versions in the controller environment, unless required libraries do not support these Python versions. The :ref:`Steering Committee <steering_responsibilities>` can grant other exceptions on a case-by-case basis. | ||||||
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.
Suggested change
i would replace 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. Could this all be simplified to: Then we avoid having to define 'eligible' python versions entirely have less for the reader to ...read and understand. 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. I think it's too late to make larger changes to this, since we already started voting on it - see ansible-community/community-topics#268. Right now we should stick so simple wording changes or small formulation changes that don't change meanings. Also if we simplify it, it should be
Also this needs to be repeated six times in the text (sometimes with Controller Python, sometimes with Target Python). I'm not sure that really improves readability. 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. let's merge it as voted and then think about simplifications. I think the requirements (especially Python-related) look hard to read and understand overall (at least to me). But let's do it in a dedicated separate PR |
||||||
|
||||||
The collection MUST document all eligible controller Python versions that are not supported in the controller environment. See :ref:`coll_python_docs_req` for details. | ||||||
|
||||||
Other environment | ||||||
~~~~~~~~~~~~~~~~~ | ||||||
|
||||||
In the other environment, collections MUST support Python 2 (version 2.7) and Python 3 (Version 3.6 and higher), unless required libraries do not support these versions. Collections SHOULD also support Python v2.6 and v3.5 if all required libraries support this version. | ||||||
Collections MUST support all eligible controller Python versions in the other environment, unless required libraries do not support these Python versions. The :ref:`Steering Committee <steering_responsibilities>` can grant other exceptions on a case-by-case basis. | ||||||
|
||||||
.. note:: | ||||||
Collections SHOULD support all eligible target Python versions in the other environment. | ||||||
|
||||||
If the collection does not support Python 2.6 and/or Python 3.5 explicitly then take the below points into consideration: | ||||||
The collection MUST document all eligible target Python versions that are not supported in the other environment. See :ref:`coll_python_docs_req` for details. | ||||||
|
||||||
- Dropping support for Python 2.6 in the other environment means that you are dropping support for RHEL6. RHEL6 ended full support in November, 2020, but some users are still using RHEL6 under extended support contracts (ELS) until 2024. ELS is not full support; not all CVEs of the python-2.6 interpreter are fixed, for instance. | ||||||
|
||||||
- Dropping support for Python 3.5 means that Python 2.7 has to be installed on Ubuntu Xenial (16.04) and that you have to support Python 2.7. | ||||||
.. note:: | ||||||
|
||||||
Also, note that dropping support for a Python version for an existing module/plugin is a breaking change, and thus requires a major release. A collection MUST announce dropping support for Python versions in their changelog, if possible in advance (for example, in previous versions before support is dropped). | ||||||
Note that dropping support for a Python version for an existing module/plugin is a breaking change, and thus requires a major release. A collection MUST announce dropping support for Python versions in their changelog, if possible in advance (for example, in previous versions before support is dropped). | ||||||
|
||||||
.. _coll_python_docs_req: | ||||||
|
||||||
Python documentation requirements | ||||||
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | ||||||
|
||||||
* If everything in your collection supports the same Python versions as the collection-supported versions of ansible-core, you do not need to document Python versions. | ||||||
* If everything in your collection supports all eligible controller/target Python versions, you do not need to document supported Python versions. | ||||||
* If your collection does not support those Python versions, you MUST document which versions it supports in the README. | ||||||
* If most of your collection supports the same Python versions as ansible-core, but some modules and plugins do not, you MUST include the supported Python versions in the documentation for those modules and plugins. | ||||||
|
||||||
|
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.
eligible
with a synonym if possible?allowed
oracceptable
maybe? can be easier to understand for not native speakersFor example, if your collection supports ansible-core version A (supports Python ...) and ansible-core version B (supports Python ...), the acceptable controller Python version is ...
?. Same for target versions.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.
Hmm, 'acceptable' could be ok, or maybe 'admissible'. I'd like to hear some more comments on this, also from native english speakers :)
About adding an example: there is already one starting directly after the diff that GitHub shows ends, in the part on documentation requirements. Is that one enough, or should we have another one here?
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.
I'd agree with the word
acceptable
thanallowed
I will also like to suggest that this part of the sentence "An eligible controller Python version for a collection is a Python version that is supported on the controller side by at least one ansible-core version that the collection supports."
could be rewritten as "An acceptable controller Python version for collection is a Python version that is supported on the controller side by at least one ansible-core version that the collection supports."
Similarly, for target versions, take off the
a
beforecollection
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.
Are you sure that removing the
a
beforecollection
is fine? That sounds really wrong to me.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.
about the examples, if there are any, it's OK
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.
acceptable
and/ordesirable
sounds like good options here.I agree, removing
a
beforecollection
does not sound right."An acceptable controller Python version for a collection is the Python version(s) that is supported on the controller side by at least one ansible-core version that the collection supports."
the Python version(s)
does this change sound more accurate?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.
I'll throw another one into the mix and say
suitable
because it has less negative/weird connotation thanacceptable
anddesirable
. Alsoeligible
doesn't quite fit imo. Something can be eligible without really being suitable.