Update Python requirements for Ansible inclusion requirements document #281

felixfontein · 2023-08-10T19:37:20Z

See ansible-community/community-topics#251, in particular ansible-community/community-topics#251 (comment).

Closes #68.

felixfontein · 2023-08-10T19:38:21Z

This doesn't yet have an example, but there's already one a bit further down in the document.

gotmax23

This looks good to me other than my one language suggestion, but I may have other feedback after I think about it more. Thanks for working on it!

docs/docsite/rst/community/collection_contributors/collection_requirements.rst

Co-authored-by: Maxwell G <[email protected]>

Andersson007 · 2023-08-17T08:50:03Z

docs/docsite/rst/community/collection_contributors/collection_requirements.rst

@@ -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.


Suggested change

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.

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.

can we replace the word eligible with a synonym if possible? allowed or acceptable maybe? can be easier to understand for not native speakers

can we add an example? something like For 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.

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?

I'd agree with the word acceptable than allowed

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 before collection

Are you sure that removing the a before collection is fine? That sounds really wrong to me.

about the examples, if there are any, it's OK

acceptable and/or desirable sounds like good options here.

I agree, removing a before collection 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?

I'll throw another one into the mix and say suitable because it has less negative/weird connotation than acceptable and desirable. Also eligible doesn't quite fit imo. Something can be eligible without really being suitable.

Andersson007 · 2023-08-17T08:51:02Z

docs/docsite/rst/community/collection_contributors/collection_requirements.rst

 .. _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. 


Suggested change

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.

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.

i would replace eligible also here (see the comment above) and everywhere with a synonym.

Could this all be simplified to:
Collections MUST support all Python versions listed in refansible_core_support_matrix for all ansible-core versions that the collection supports. The exception is if required libraries for the collection do not support these Python versions. Steering committee blablabla...

Then we avoid having to define 'eligible' python versions entirely have less for the reader to ...read and understand.
`

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

Collections MUST support all Python versions listed in :ref:ansible_core_support_matrix in the Controller Python column for all ansible-core versions that the collection supports.

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.

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

cidrblock

the core matrix refers to versions as "supported", maybe that is the language to use?

felixfontein · 2023-08-31T18:51:47Z

This PR was approved by the vote in ansible-community/community-topics#268, which accepted it (ansible-community/community-topics#251 (comment)).

patchback · 2023-08-31T18:52:03Z

Backport to stable-2.14: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-2.14/08263fc9b7e23fc524f5ea560a843e61001f8387/pr-281

Backported as #348

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback · 2023-08-31T18:52:03Z

Backport to stable-2.15: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-2.15/08263fc9b7e23fc524f5ea560a843e61001f8387/pr-281

Backported as #349

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

felixfontein · 2023-08-31T18:52:50Z

Thanks everyone! I guess we can use ansible-community/community-topics#251 to discuss improved wording, or if someone wants they can also create a PR for that and we can discuss in there.

#281) * Update Python requirements for Ansible inclusion requirements document. * Avoid passive voice. Co-authored-by: Maxwell G <[email protected]> --------- Co-authored-by: Maxwell G <[email protected]> (cherry picked from commit 08263fc)

#281) (#348) Co-authored-by: Maxwell G <[email protected]> (cherry picked from commit 08263fc) Co-authored-by: Felix Fontein <[email protected]>

#281) (#349) Co-authored-by: Maxwell G <[email protected]> (cherry picked from commit 08263fc) Co-authored-by: Felix Fontein <[email protected]>

github-actions bot added sc_approval This PR requires approval from the Ansible Community Steering Committee needs_triage Needs a first human triage before being processed. labels Aug 10, 2023

felixfontein mentioned this pull request Aug 10, 2023

[Associated vote ended on 2023-08-30] Update Python requirements for collections included in Ansible ansible-community/community-topics#251

Closed

Update Python requirements for Ansible inclusion requirements document.

327240e

felixfontein force-pushed the python-2 branch from e7cb299 to 327240e Compare August 10, 2023 19:38

gotmax23 reviewed Aug 10, 2023

View reviewed changes

docs/docsite/rst/community/collection_contributors/collection_requirements.rst Outdated Show resolved Hide resolved

docs/docsite/rst/community/collection_contributors/collection_requirements.rst Outdated Show resolved Hide resolved

oraNod removed the needs_triage Needs a first human triage before being processed. label Aug 14, 2023

Avoid passive voice.

47cf6b4

Co-authored-by: Maxwell G <[email protected]>

felixfontein mentioned this pull request Aug 16, 2023

Community Working Group Meeting Agenda 2023 ansible/community#679

Closed

samccann mentioned this pull request Aug 16, 2023

Update docs to use control node instead of controller #293

Closed

Andersson007 reviewed Aug 17, 2023

View reviewed changes

samccann mentioned this pull request Aug 24, 2023

Update Python requirements for Ansible inclusion requirements document #68

Closed

cidrblock approved these changes Aug 30, 2023

View reviewed changes

felixfontein marked this pull request as ready for review August 31, 2023 18:51

felixfontein requested a review from a team as a code owner August 31, 2023 18:51

felixfontein merged commit 08263fc into ansible:devel Aug 31, 2023
3 checks passed

felixfontein added backport-2.15 Automatically create a backport for the stable-2.15 branch backport-2.14 Automatically create a backport for the stable-2.14 branch labels Aug 31, 2023

felixfontein deleted the python-2 branch August 31, 2023 18:52

patchback bot mentioned this pull request Aug 31, 2023

[PR #281/08263fc9 backport][stable-2.14] Update Python requirements for Ansible inclusion requirements document #348

Merged

patchback bot mentioned this pull request Aug 31, 2023

[PR #281/08263fc9 backport][stable-2.15] Update Python requirements for Ansible inclusion requirements document #349

Merged

zukizukizuki mentioned this pull request Sep 3, 2023

Replace controller with control node #352

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Python requirements for Ansible inclusion requirements document #281

Update Python requirements for Ansible inclusion requirements document #281

felixfontein commented Aug 10, 2023 •

edited

Loading

felixfontein commented Aug 10, 2023

gotmax23 left a comment

Andersson007 Aug 17, 2023

felixfontein Aug 17, 2023

Dule-martins Aug 22, 2023

felixfontein Aug 23, 2023

Andersson007 Aug 23, 2023

anweshadas Aug 23, 2023

oraNod Aug 23, 2023

Andersson007 Aug 17, 2023

samccann Aug 22, 2023

felixfontein Aug 23, 2023

Andersson007 Aug 23, 2023 •

edited

Loading

cidrblock left a comment

felixfontein commented Aug 31, 2023

patchback bot commented Aug 31, 2023 •

edited

Loading

patchback bot commented Aug 31, 2023 •

edited

Loading

felixfontein commented Aug 31, 2023

		@@ -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.

	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.
	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.

Update Python requirements for Ansible inclusion requirements document #281

Update Python requirements for Ansible inclusion requirements document #281

Conversation

felixfontein commented Aug 10, 2023 • edited Loading

felixfontein commented Aug 10, 2023

gotmax23 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

Choose a reason for hiding this comment

Andersson007 Aug 23, 2023 • edited Loading

Choose a reason for hiding this comment

cidrblock left a comment

Choose a reason for hiding this comment

felixfontein commented Aug 31, 2023

patchback bot commented Aug 31, 2023 • edited Loading

Backport to stable-2.14: 💚 backport PR created

patchback bot commented Aug 31, 2023 • edited Loading

Backport to stable-2.15: 💚 backport PR created

felixfontein commented Aug 31, 2023

felixfontein commented Aug 10, 2023 •

edited

Loading

Andersson007 Aug 23, 2023 •

edited

Loading

patchback bot commented Aug 31, 2023 •

edited

Loading

patchback bot commented Aug 31, 2023 •

edited

Loading