powa-team / powa-web

PoWA user interface
http://powa.readthedocs.io/
74 stars 31 forks source link

Drop support for python < 3.6 #228

Open pgiraud opened 1 week ago

rjuju commented 1 week ago

I'm quite opposed to this. What problem does it solve? It only creates new problem, as the target for powa is not a web developer running some distro with all the packages in bleeding edge version but companies running postgres on sometimes somewhat legacy systems, usually without any possibility to upgrade anything. I'm still annoyed that we don't support python 2 anymore but ok it's really dead. But I'm pretty sure that no one is going to thank us for dropping support for python 3.9-. Even our demo still uses an older version than that.

rjuju commented 1 week ago

again, why aggressively dropping support for python versions that users might be using?

I spent a bit of time trying to figure out what versions are now supported, since you didn't document it. It seems to me that python 3.3 would work. It looks like it's only broken by ruff that adds trailing commas everywhere. I'd rather remove that behavior and restore support down to python 3.3.

pgiraud commented 1 week ago

I don't think python 3.3 should fail on trailing comas. Can you please give example of code that breaks compatibility with python 3.3?

rjuju commented 1 week ago

You can reproduce easily by spawning a python:3.3 container and bind mounting powa-web. Doing so:

root@236b65caeb0f:/usr/local/src/powa/powa-web# ./run_powa.py 
Traceback (most recent call last):
  File "./run_powa.py", line 4, in <module>
    from powa import make_app
  File "/usr/local/src/powa/powa-web/powa/__init__.py", line 132
    **kwargs,
            ^
SyntaxError: invalid syntax

after removing the trailing comma it fails somewhere else, so I'm assuming it's the problem:

root@236b65caeb0f:/usr/local/src/powa/powa-web# ./run_powa.py 
Traceback (most recent call last):
  File "./run_powa.py", line 4, in <module>
    from powa import make_app
  File "/usr/local/src/powa/powa-web/powa/__init__.py", line 24, in <module>
    from powa.collector import (
  File "/usr/local/src/powa/powa-web/powa/collector.py", line 9, in <module>
    from powa.dashboards import MetricGroupDef
  File "/usr/local/src/powa/powa-web/powa/dashboards.py", line 529
    **kwargs,
            ^
SyntaxError: invalid syntax
pgiraud commented 6 days ago

Indeed, the following is invalid for Python < 3.6 (see https://bugs.python.org/issue9232).:

def foo(bar, dude,): # valid
    [...]

def foo(bar, **kwargs,): # invalid
    [...]

What problem does it solve?

First, I would like to apologize for the commits that introduced the formatting changes with Ruff. I'm sorry I didn't pay enough attention to the target python version, didn't test meticulously and more importantly didn't document the change.

By the way, it's not really a Ruff problem. I could have used Black with similar results.

I doesn't really solve any problem. Not in the way the tool actually works. I was just trying to make sure that the code style is consistent and modern for the sake of developers but not only. See below.

Why aggressively dropping support for python versions that users might be using?

How do we know that the current users won't be able to use PoWA because of compatibility issues? This seems very hypothetical. I'd be curious to know how many users will want to upgrade PoWA and are not able to install a relatively recent version of Python (> 3.6).

The target for powa is not a web developer running some distro with all the packages in bleeding edge version

Python follows a clear release cycle. Just like PostgreSQL does. https://devguide.python.org/versions/

Python 3.5 was released in 2015-09-13 and reached end of life in 2020-09-30. While I agree that 3.9 was a bit too restrictive, I think keeping support for 3.6 is reasonable.

Keeping PoWA compatible with old versions of Python gives the project a dusty image, which can have the effect of slowing down outside contributions, whether new or old. Potential contributors may already be reluctant to contribute to the project if the code isn't modernized or if proposals for modernization are rejected.

If the Python language evolves, it's for the good of its users, who can then write more secure/maintainable/readable/... code. If our project's code itself doesn't evolve, we are leaving developers to work in an environment that's uncomfortable because it's not very reassuring, or even counter-productive given that they have to constantly focus on compatibility to the detriment of the rest, i.e. what really matters. Missing tests is also a bad sign.

Don't we want to see new motivated contributors join the project? Or do we want to keep the compatibility for a hypothetical user base?

Also, we probably don't want users to run PoWA on versions of Python that are not maintained for security reasons. Generally, it's a good thing to advise users to keep their system up-to-date. And up-to-date doesn't necessarily mean bleeding edge.

Could we please at least accept that Python 3.6 (preferably 3.7) is the minimal version PoWA can be run onto? That would already be an appreciated step forward.

rjuju commented 6 days ago

Indeed, the following is invalid for Python < 3.6 (see https://bugs.python.org/issue9232).

Ah I see, the difference is subtle (at least for a non python developer like me) :)

First, I would like to apologize for the commits that introduced the formatting changes with Ruff. I'm sorry I didn't pay enough attention to the target python version, didn't test meticulously and more importantly didn't document the change.

No worries, fortunately the UI in this project is quite disconnected from the rest so it's really easy to release new versions frequently and it's not really a problem to apply them on the client side (assuming they do upgrade).

By the way, it's not really a Ruff problem. I could have used Black with similar results.

Yes totally agreed. I'm assuming it's just implementing some PEP recommendations.

How do we know that the current users won't be able to use PoWA because of compatibility issues? This seems very hypothetical. I'd be curious to know how many users will want to upgrade PoWA and are not able to install a relatively recent version of Python (> 3.6).

In my view that should be the opposite: how can you guarantee that no one will ever need to update powa using some older python version?

But to answer your question, after working for decades around databases, reading discussion on the various mailing list and so on I see all the time people using very old versions of postgres, and I'm not talking about only the minor version of postgres. And usually those people just stick with the same version of what they originally installed: postgres, OS and everything else. That's why at least RHEL has very long extended support.

I agree that cloud providers and hosted solutions won't have this problem, but they also won't use or provide powa either.

Python follows a clear release cycle. Just like PostgreSQL does. https://devguide.python.org/versions/

Python 3.5 was released in 2015-09-13 and reached end of life in 2020-09-30. While I agree that 3.9 was a bit too restrictive, I think keeping support for 3.6 is reasonable.

Note that powa-archivist still support postgres 9.5, released in 2016 and EOLed in 2021. That's why I think that the front-end should be compatible with version of python that roughly matches the same interval.

Keeping PoWA compatible with old versions of Python gives the project a dusty image, which can have the effect of slowing down outside contributions, whether new or old. Potential contributors may already be reluctant to contribute to the project if the code isn't modernized or if proposals for modernization are rejected.

In my experience if people don't want to contribute to something they will always find good reasons not to. On the other hand if they do want to contribute they're unlikely to be driven off by something like that, especially if the CI is there to catch syntax errors / older version compatibility.

If the Python language evolves, it's for the good of its users, who can then write more secure/maintainable/readable/... code. If our project's code itself doesn't evolve, we are leaving developers to work in an environment that's uncomfortable because it's not very reassuring, or even counter-productive given that they have to constantly focus on compatibility to the detriment of the rest, i.e. what really matters. Missing tests is also a bad sign.

Don't we want to see new motivated contributors join the project? Or do we want to keep the compatibility for a hypothetical user base?

Whether we raise the minimum python version or not will not change the fact that the code will still be complex, full of over lengthy queries that have to deal with multiple versions of postgres, multiple combinations of extensions and so on. That's far more likely to discourage anyone from working on this project than not being able to use whatever new syntax that would now be allowed. Unfortunately what this is doing is actively preventing me from contributing to this project since I don't really understand newer python code (I'm already entirely unable to do anything on the js side). I'm not a python developer (which likely explains a lot of the weird code here) and haven't used python at work for years, so I don't really have any opportunity (or time) to keep up with it.

Also, we probably don't want users to run PoWA on versions of Python that are not maintained for security reasons. Generally, it's a good thing to advise users to keep their system up-to-date. And up-to-date doesn't necessarily mean bleeding edge.

While I totally agree that people shouldn't run EOL / unmaintained code, I don't think that it's our place to force them to update. Again, in my experience people don't run scary old versions of stuff because they like it, but because they simply don't have a choice. So if they happened to want/need powa, preventing them from using it would not help :(

Could we please at least accept that Python 3.6 (preferably 3.7) is the minimal version PoWA can be run onto? That would already be an appreciated step forward.

I'm ok with 3.6 (preferably), as long as you're the one dealing with the hypothetical annoyed users.

pgiraud commented 3 days ago

PR updated. Can you please have look? After doing a 5.0.1 release with the different fixes, of course.