-
Notifications
You must be signed in to change notification settings - Fork 20
framework+rdm: establish Python support policy #109
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
base: master
Are you sure you want to change the base?
Conversation
849892b to
f7f0923
Compare
| The current (at time of writing) scenario of RDMv13 is illustrative of this "periods of overlap with deprecated Python versions" section. Note however that Python 3.10 would have been the next smallest | ||
| version, but it was never officially supported by InvenioRDM, so 3.11 would probably be the next minimum supported version (per the second option). |
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.
The RDMv13 and Python 3.9 situation is also slightly different because this policy was/is not in effect at time of v13 release and transition + there is the desire from CERN to keep 3.9 support until they are off of it.
A temporary solution would then be:
- keep minimum python required version to 3.9 on RDM modules for now until the decided date where we can switch to policy.
- have
[3.9, 3.14]as test range in tests-python.yml under the branchesmasterandmaint-v13for now until decided date where we can switch. - have
[3.14]in the wings until can merge (when decided switch time) and be what you get withmaster - for framework modules, on a per module basis (or new
tests-python-framework.yml), have[3.9, 3.10, 3.14]as the test range in preparation for 3.10 becoming their minimal.
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.
Transition (v13 - 3.9) example change for a framework module: inveniosoftware/invenio-base#198
Transition (v13 - 3.9) example change for an rdm module: inveniosoftware/invenio-rdm-records#2190
83e1451 to
edee833
Compare
|
|
||
| The Python support policy outlined above leads to a scenario where a newly released InvenioRDM and the previous, but still supported for 6 months per [RDM support policy](https://inveniordm.docs.cern.ch/releases/maintenance-policy/), do not share the same mimimum required Python version for that overlap period. This could be a rare occurrence (e.g., every 5 years), but it has to be accounted for. | ||
|
|
||
| As is, it would mean that during that period of time, any fix would have to be compatible with the previous minimum Python version — a different and unsupported (by Python community) one. |
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.
This is taking the "first option" mentioned in previous version of this RFC:
Maintainers continue for 6 months to make fixes and backports that are compatible with the previous minimum Python version. This means being more careful about fixes and it doesn't promote adoption of a secure/supported version of Python. It is simpler because it doesn't require anything else. Past the 6 months mark, all the new features of the new minimum Python version can be adopted (and all deprecation workarounds that can be dropped are).
|
|
||
| Invenio-wide modules like invenio-app,-search... (as opposed to InvenioRDM-specific ones like invenio-records-resources,-rdm-records ...) are not driven by the InvenioRDM cadence and should be even more stable. They should keep supporting the minimum Python version they did and be independent of RDM. | ||
|
|
||
| For those, the lowest but still supported Python version needs to be supported. They must not be incompatible with the most recent Python version. |
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.
Currently those often have minimum Python versions that are long deprecated. This proposal pushes to make new versions that raise their minimum Python versions. A new version will help users be able to shield themselves from any breaking change required.
| This policy applies to the InvenioRDM specific modules: | ||
|
|
||
| - invenio-administration | ||
| - invenio-app-rdm | ||
| - invenio-audit-logs | ||
| - invenio-banners | ||
| - invenio-checks | ||
| - invenio-collections | ||
| - invenio-communities | ||
| - invenio-drafts-resources | ||
| - invenio-github/vcs | ||
| - invenio-jobs | ||
| - invenio-notifications | ||
| - invenio-pages | ||
| - invenio-rdm-records | ||
| - invenio-records-permissions | ||
| - invenio-records-resources | ||
| - invenio-requests | ||
| - invenio-sitemap | ||
| - invenio-stats | ||
| - invenio-users-resources | ||
| - invenio-vocabularies | ||
|
|
||
| As InvenioRDM drives the creation of new modules, those should be considered to fall under this category by default. Exceptions will be indicated. |
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.
@slint double-check that list (may include too much) and that note about new modules (may actually be outright wrong default 😅 ).
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.
After discussing a bit with @kpsherva, there are some modules that we should exclude, since InvenioILS (and others) are using (or planning to use) them.
Just from looking at dependencies in invenio-app-ils, the following modules should be removed from the above list:
invenio-stats(also used by CDS-Videos, and CERN OpenData)invenio-pages(used by InenioILS). Some upstream dependencies that are also part of the above list:invenio-administrationinvenio-records-resourcesinvenio-records-permissions
invenio-banners(used by InvenioILS)
An alternative is to also enforce Python v3.14 as the minimum version for InvenioILS (for the same reasons we do it for InvenioRDM), but there's a capacity/resources gap at this moment for implementing this. In that case, we could update the timeline below accordingly.
slint
left a comment
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.
Thanks for putting this down in writing, I really like how this is forming :)
Had a go at the "core" part of the RFC regarding supported versions across Invenio{,RDM,ILS}. I didn't have time/stamina to go through the transition periods, but I'll have a go again tomorrow.
| This policy applies to the InvenioRDM specific modules: | ||
|
|
||
| - invenio-administration | ||
| - invenio-app-rdm | ||
| - invenio-audit-logs | ||
| - invenio-banners | ||
| - invenio-checks | ||
| - invenio-collections | ||
| - invenio-communities | ||
| - invenio-drafts-resources | ||
| - invenio-github/vcs | ||
| - invenio-jobs | ||
| - invenio-notifications | ||
| - invenio-pages | ||
| - invenio-rdm-records | ||
| - invenio-records-permissions | ||
| - invenio-records-resources | ||
| - invenio-requests | ||
| - invenio-sitemap | ||
| - invenio-stats | ||
| - invenio-users-resources | ||
| - invenio-vocabularies | ||
|
|
||
| As InvenioRDM drives the creation of new modules, those should be considered to fall under this category by default. Exceptions will be indicated. |
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.
After discussing a bit with @kpsherva, there are some modules that we should exclude, since InvenioILS (and others) are using (or planning to use) them.
Just from looking at dependencies in invenio-app-ils, the following modules should be removed from the above list:
invenio-stats(also used by CDS-Videos, and CERN OpenData)invenio-pages(used by InenioILS). Some upstream dependencies that are also part of the above list:invenio-administrationinvenio-records-resourcesinvenio-records-permissions
invenio-banners(used by InvenioILS)
An alternative is to also enforce Python v3.14 as the minimum version for InvenioILS (for the same reasons we do it for InvenioRDM), but there's a capacity/resources gap at this moment for implementing this. In that case, we could update the timeline below accordingly.
rfcs/rdm-0109-python-versions.md
Outdated
|
|
||
| ### InvenioRDM — v14 and onwards | ||
|
|
||
| Starting with InvenioRDM v14, the idea is to set the minimum required Python version to a specific version and keep it so until the InvenioRDM release of the year where that Python version is outdated OR until maintainers decide to change that minimum version. This version will be tested against directly to ensure backwards compatibility over time. The maximum officially supported Python version will be the latest released version of Python at time of release of InvenioRDM. This applies to RDM specific modules and RDM instances. |
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.
| Starting with InvenioRDM v14, the idea is to set the minimum required Python version to a specific version and keep it so until the InvenioRDM release of the year where that Python version is outdated OR until maintainers decide to change that minimum version. This version will be tested against directly to ensure backwards compatibility over time. The maximum officially supported Python version will be the latest released version of Python at time of release of InvenioRDM. This applies to RDM specific modules and RDM instances. | |
| Starting with InvenioRDM v14, the idea is to set the minimum required Python version to a specific version and keep it so until the InvenioRDM release of the year where that Python version is outdated OR until maintainers decide to change that minimum version. This version will be tested to ensure backwards compatibility over time. The maximum officially supported Python version will also be the selected version of Python at the time of the latest release of InvenioRDM. This applies to RDM specific modules and RDM instances. |
- First proposed change is just wording/phrasing
- Second change is about the maximum version: officially I would only support the minimum version we select (i.e. one version to rule them all). We should configure future versions in CI as allowed failures to understand how we're doing on bugs/deprecations/performance.
Meta: I think we should adopt something like "Semantic Line Breaks" across all our docs and prose meant to be reviewed in a PR-like fashion 😅
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.
Couple of thoughts re. the "one python version to rule them all":
[Edit the next set of comments touch on this so this may already be passé]:
- What I am thinking is that in development (i.e. in the CI test range) we want to include the latest released Python version, so that we can see coming change ahead and prepare for it. Then, it effectively results in InvenioRDM supporting that coming version by the time we release it. It doesn't mean there is an official docker-invenio image for that version, but it's "supported".
- A single official version would be nice maintenance-wise though.
- A single supported version is likely to be too constrictive for broad adoption: sometimes it's not something a team has easy control over if they are not using containers, sometimes folks (looking at @utnapischtim) want to run a more recent version for perceived gains/keep ahead of the game.
- A "tiered-support" approach — this is the main actively supported version and these are the observed to be supported versions (instead of 3 versions communicated as equally supported, communicate one more than the others) — could exist but would be hard to communicate and I doubt it would moderate expectations or work on our side in practice
- I think it boils down to how much effort supporting multiple versions is. My sense is that it's typically not too bad with little spikes of more effort here and there for a nicer adoption benefit (maybe that doesn't take some things into account though). Input from others welcome!
Haha "Semantic Line Breaks" sounded like a good idea, but then I considered how I write and it might be too much for me. I already obsess about the words and tone and meaning... having to obsess about line breaks too might just prevent me from writing in a time-efficient manner hahaha.
rfcs/rdm-0109-python-versions.md
Outdated
|
|
||
| For InvenioRDM v14, the target minimum version is Python 3.14 (it turns out nice that both have 14 in the version number). Here is how Python support would look like pre and post release of that version: | ||
|
|
||
| | Timeline | Context | Min. Python version required | Max. Python version supported | |
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 think it's crucial what exact wording we choose and how we interpret it:
- "supported version": we commit to prioritizing and fixing any bugs discovered in this version
- "tested (or compatible) version": we test these versions as part of CI, and users can use them, but at their own risk. Bugs or broken builds in these versions will not be prioritized
- "required (minimum) version": I wouldn't actually use this wording...
An alternative is to introduce a "Tier" approach, like e.g. CPython's PEP11 defines:
- Tier 1: bugs are prioritized, CI runs on all PRs, broken builds block releases
- Tier 2: tested in CI, but bugs are lower priority, doesn't block releases
I think this a bit worse though compared to "supported/tested (or compatible)", since it requires an explanation of what each Tier entails.
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.
haha I hadn't even read this comment before I wrote the one above and it aligns pretty well so that's good!
I think the "supported version" and "compatible versions" explanation just given could work... It's just that given that a broken CI is high priority (in my books), I don't know if there is a big difference then between a supported and compatible version in practice...
The "Min. Python version required" refers to the value placed in requires-python = (pyproject.toml) or [requires] python_version = (Pipfile) or python_requires = (setup.cfg) . That could be renamed to "Officially supported Python version" in the first column and "Maximum compatible version" if we go that way. I'll change as such for now, but we should get more input to solidify the choice.
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.
...It's just that given that a broken CI is high priority...
For supported versions of course, we wouldn't merge a PR with broken tests.
For the "compatible" versions, I would be more flexible though: we would still try to fix things, since it would mean less and more incremental work for us when we eventually perform the version bump. But if the build is too complex to fix (or maybe even broken because of how new the Python version is and thus dependencies have limited support for it), I would just merge and reprioritize (or even wait until someone running this newer Python version actually needs to fix it).
Now that I read my explanations again, the "compatible" wording itself doesn't help much... I think what I describe sounds more like "experimental" or "preview"?
The "Min. Python version required" refers to the value placed in
requires-python =(pyproject.toml) or[requires] python_version =(Pipfile) orpython_requires =(setup.cfg) .
Ok, that makes a lot of sense. It's sort of a direct consequence of having a specific supported minimum version.
edee833 to
167c924
Compare
|
I pushed some edits. Areas of discussion left (I can be swayed and it would be nice to hear from others too):
I will recheck at the end of my day else when I come back from holidays (I am fine if a decision about this is done while I am away. Either way it shakes out, there is nothing I strongly disagree with). |
This is actually my summary of the discussions maintainers had. I've separated out my own comments / thoughts, so this should be understood.
Also in the below text "supported Python version" should be read as "officially supported by the Python foundation/community"