canonical / data-platform-libs

A collection of charm libraries curated by the Data Platform Team
https://charmhub.io/data-platform-libs
Apache License 2.0
10 stars 9 forks source link

[DPE-3906][BUGFIX] dict implementation not supporting Python 3.9> (issue #152) #153

Closed juditnovak closed 6 months ago

juditnovak commented 6 months ago

URGENT FIX -- Python 3.9> compatiblity broken

Once again I had to remember the hard way: data_interfaces MUST be strongly backwards compatible.

Both in terms of code and in terms of Python versions, etc.

Adding generics to the dict implementation parent class broke Python 3.9> compatibility.

The proposed fix executes correctly on earlier Python versions as well: https://github.com/canonical/data-platform-libs/actions/runs/8559046518

marcoppenheimer commented 6 months ago

@deusebio - It's because of old CI that's being ran on 20.04. I don't think we do (or should) support 20.04 for our latest work anymore so I don't think this PR is needed.

juditnovak commented 6 months ago

@marceloneppel , @deusebio I don't think we should limit our lib users in their choice of Ubuntu/Python version. We have many charms (like wordpress or indico). Neither we should deprive latest enhancements (or bugfixes) from them.

We can have pretty, up-to-date code in our charms where we're in control of Ubuntu/Python versions, where we know what our clients need/use, etc. But generic interfaces that we provide should be more allowing than that.

As explained in the related issue, the problem was raised on the public Matrix channel by the Indico charm team: https://matrix.to/#/!BukWfnyOTgQSKAxdtT:ubuntu.com/$X0SV2M_Xb-MYL3gOAmXJzuFVSOeFQc9MKXBIwDUwA6w?via=ubuntu.com&via=matrix.org

deusebio commented 6 months ago

To me, EoL is a good enough reason to stop making sure code is backward compatible with that version. Otherwise how do we draw the line between which versions you want to be compatible with?

Anyhow, I would probably add a matrix on the CI (only unittests in my opinion) against the different python versions we support. Of course, outside of this PR if the other team is blocked and we need to merge this asap.

juditnovak commented 6 months ago

Let's be honest -- we introduced a breaking change ;-) While indeed we may want to consider what level of backwards compatibility should our libs support -- equally we should avoid incompatible/breaking changes :-) At least across minor versions

deusebio commented 6 months ago

Let's be honest -- we introduced a breaking change ;-)

Yes, totally! Never said otherwise. Also EoL is not here just yet. I actually approved straight away. But this has highlighted to me that we could/should provide more clarity and policies around supported versions. I'm going to bring this up on a manager sync next week.

juditnovak commented 6 months ago

@deusebio Yes, I saw your "instant approval" -- thank U very much 4 that :-)

juditnovak commented 6 months ago

Adding a summary of team discussion on this matter, with @delgod involved.

Some of our products (like PGbouncer) are supporting 20.04. We must respect backwards compatibility with applications using our products.