Skip to content
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

Clarify stance on Python micro version support #278

Closed
scop opened this issue Aug 19, 2019 · 17 comments · Fixed by #280
Closed

Clarify stance on Python micro version support #278

scop opened this issue Aug 19, 2019 · 17 comments · Fixed by #280

Comments

@scop
Copy link
Member

scop commented Aug 19, 2019

Context

ADR-0002 currently specifies the support cycle for Python major.minor versions. This is not enough, as micro versions come with changes where ones introduced in newer don't work with earlier ones. For example 3.6.0 is quite different from 3.6.9.

The codebase currently checks for >= 3.6.0.

There are already bugs related to use of python 3.6.x (x > 0) features, for example home-assistant/core#26030

People appear to have mixed opinions what the currently stated policy means:

Proposal

Explicitly specify the minimum supported micro version somewhere. Amend ADR-0002 to mention that. Make sure that the version check in the code matches that, and make sure CI is using the oldest supported version.

For example, if at the moment the minimum supported version is 3.6.0:

  • check in code is correct, no changes needed
  • make sure Azure and Travis configs are using 3.6.0 based images (they may use some additional ones of course). (Also Tox if doable, but I'm afraid not.)

And additionally, ADR-0002 should be amended to say either that:

  • the minimum micro version varies and is specified by the version check in HA code, or
  • the minimum micro version is always .0.

Consequences

Removes ambiguity from developers and end users.

@pvizeli
Copy link
Member

pvizeli commented Aug 19, 2019

Thanks for the issue.

My 50cent:
Every 3.6.x are not only bugfixes. They fix also high-security CVE. It is like with every software, you should update your computer and software. That is going important as like for 10 years. Fact is, that most of the packages system can't address the current cybersecurity like it needs to be. Containers come there and help out there a lot.

We support the latest 2 releases and we should only support feature from 3.x.0 for sure but we should not run such old version on our systems with have open over 36 CVEs.

@scop scop changed the title Clarify stance on Python minor version support Clarify stance on Python micro version support Aug 19, 2019
@balloob
Copy link
Member

balloob commented Aug 20, 2019

When we supported 3.5.3, we said we explicitly support Python 3.5.3 and up. This was based on a required feature inside asyncio. Our current ADR does indeed not specify what version of Python. I think that we should aim for the .0 release.

@pvizeli this issue is not about what we run in hass.io, we obviously run the latest that we feel comfortable with (usually .2+). This is about what we want to minimum support with the Home Assistant codebase. If it is 3.6.0, we should make sure our tests/linting use 3.6.0 so that we can catch code that assumes more modern features.

@andrewsayre
Copy link
Member

So 3.6.0 is actually older than 3.5.3 was... do we have any reason not to support the latest patch release -2, just like minor version?

@balloob
Copy link
Member

balloob commented Aug 20, 2019

I like that idea.

@scop
Copy link
Member Author

scop commented Aug 20, 2019

One problem with the "latest patch -2" approach is that while Python micro version releases do come out fairly often, (most?) distributions do not update their micro version; they backport security patches instead. In my opinion it would be better if HA would stick with whatever micro version within a major version provides the necessary/desired features, and not make it a moving target.

@ties
Copy link

ties commented Aug 20, 2019

One problem with the "latest patch -2" approach is that while Python micro version releases do come out fairly often, (most?) distributions do not update their micro version; they backport security patches instead. In my opinion it would be better if HA would stick with whatever micro version within a major version provides the necessary/desired features, and not make it a moving target.

As an example, Red Hat does security fixes on micro versions. This is Python 3.6.3 with a back-ported security patch for CVE-2019-9636.

Updating the micro version when major features are added (e.g., 3.5.3) is fine. But otherwise it may cause strange issues to chase the latest patch release.

(Do we have statistics on the number of users sticking to such an old build? If it they are negligible, it may be worth chasing the latest patch release)

@pvizeli
Copy link
Member

pvizeli commented Aug 20, 2019

I like @andrewsayre idea. Latest 2 minor release

@balloob
Copy link
Member

balloob commented Aug 20, 2019

image

@balloob
Copy link
Member

balloob commented Aug 20, 2019

Looking at this data, we should stick to the 0 releases unless there is a critical feature (like we had with 3.5.3)

@balloob
Copy link
Member

balloob commented Aug 20, 2019

But if we ever need a minimum version bump for a critical feature, we should add 2 months deprecation time.

@andrewsayre
Copy link
Member

Looking at this data, we should stick to the 0 releases unless there is a critical feature (like we had with 3.5.3)

How do you get that from the data? Isn't that showing 3.6.6 is the most used 3.6.x version?

@balloob
Copy link
Member

balloob commented Aug 20, 2019

Sure but there are still significant users that would be left out if we dropped older versions. Those squares of older versions represent ~5k users.

@andrewsayre
Copy link
Member

Got it.. though I wonder are they using an older version for a reason, or just haven’t updated, or using .0 because that’s what we say is the min version.

@scop
Copy link
Member Author

scop commented Aug 20, 2019

Good point.

Anyway, Debian is another distro that I believe doesn't bump micro version within a release. For example their stretch package is nominally at 3.5.3, but contains security fixes from at least 3.5.6. Yet, 3.5.3 wouldn't fit the latest-2 support range. (Just to illustrate, 3.5 is no longer relevant.)

BTW do we have data on 0.97+ only? The one showing a large 3.5.x install base may be skewed somewhat, because it contains old HA setups done with some "back then" 3.6.x version, and those aren't relevant wrt supportability decisions now because the only currently supported HA is 0.97. I think data for 0.97+ only would be more representative when assessing what the baseline should be at the moment. It'd still contain some skew (old pre 0.97 setups that already ran an older suitable 3.6.x or later upgraded to 0.97, so no python upgrade necessary), but possibly less.

@scop
Copy link
Member Author

scop commented Aug 26, 2019

So... what's the conclusion, or is there something still needed before this can move forward? Personally I'd still prefer picking some 3.6.x version and sticking with that unless some required features come up that would necessitate increasing x. I think either x=0 (like kind of is now) or x=1 would make sense -- some dependencies already require 3.6.1, at least websockets: python-websockets/websockets#655

@balloob
Copy link
Member

balloob commented Aug 26, 2019

I say we stick with 3.6.0. I've left a comment on the websockets PR, hoping they can fix their type.

@balloob
Copy link
Member

balloob commented Aug 29, 2019

Tried fixing the type in websockets but the author said there were a couple of places that are already 3.6.1 specific, and they don't have the resources to run every micro version, which is understandable.

I suggest that we drop Python 3.6.0 support starting 0.99 and migrate to minimal Python 3.6.1 in 0.100.

Here are all the current packages that depend on websockets:

$ pipdeptree -r -p websockets
websockets==6.0
  - aioambient==0.3.2 [requires: websockets]
  - discord.py==1.2.3 [requires: websockets>=6.0,<7.0]
  - graphql-subscription-manager==0.3.1 [requires: websockets>=6.0]
    - pyTibber==0.11.6 [requires: graphql-subscription-manager>=0.3.1]
  - hbmqtt==0.9.4 [requires: websockets]
    - tellsticknet==0.1.2 [requires: hbmqtt]
      - pytfiac==0.3 [requires: tellsticknet]
    - volvooncall==0.8.7 [requires: hbmqtt]
  - homematicip==0.10.10 [requires: websockets]
  - pylgtv==0.1.9 [requires: websockets]
  - pyTibber==0.11.6 [requires: websockets>=6.0]
  - sense-energy==0.7.0 [requires: websockets]
  - vallox-websocket-api==2.2.0 [requires: websockets>=7.0,<8.0]

balloob pushed a commit to home-assistant/core that referenced this issue Oct 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants