search

scylladb / seastar

High performance server-side application framework
http://seastar.io
Apache License 2.0
8.39k stars 1.55k forks source link

Python version for seastar-addr2line #2433

Open travisdowns opened 2 months ago

travisdowns commented 2 months ago

What is the Python version requirement for seastar-add2line?

Evidently it requires at least Python 3.6 since it uses f-strings, but I'm not sure beyond that.

I'd like to make some enhancements, e.g., type annotations but would need to know if say targeting Python 3.9 (released 8 years ago) is alright.

tchaikov commented 2 months ago

hi Travis,

e.g., type annotations but would need to know if say targeting Python 3.9 (released 8 years ago) is alright.

probably you meant python 3.6? as python 3.9 was released 4 years ago, see https://www.python.org/downloads/release/python-390/ .

it's a good question when it comes to a stand-alone script. as we don't have a setup.py or a pyproject.toml for tracking the metadata attached to this script. IMHO, it's should be safe to use features in Python 3.9 from scylla's perspective, as scylladb is now using fedora 40 is its base image. and fedora 40 comes with python 3.12. but to bump up the minimum required versions only based on scylla's needs is probably not quite a right decision making process, as seastar is separate library.

but as you pointed out Python 3.9 was released ~8~ 4 years ago, probably we can start using it?

@denesb what do you think? is it safe to start using Python 3.9 ?

nyh commented 2 months ago

@denesb what do you think? is it safe to start using Python 3.9 ?

A separate (but related) question is what will be the benefit to assume Python 3.9. addr2line is a nothing more than a small script, 390 lines of code. Will it really benefit from using new Python 3.9 features? Will "type annotations" help anything? There is the possibility that the answer is "no". As the American saying goes, "if it ain't broke, don't fix it" ;-) However, if the answer is "yes" - type annotations will help me grow addr2line from a 300-line half-working script into a fabulous 3000-line script, then, I guess, go ahead.

denesb commented 2 months ago

@denesb what do you think? is it safe to start using Python 3.9 ?

I have no idea :laughing:. I may have written this script, but I just used python features I knew about at the time and that the python version I happened to have installed supported. There was no deliberation behind this, I'm not a Pythonista.

travisdowns commented 2 months ago

@tchaikov wrote:

probably you meant python 3.6? as python 3.9 was released 4 years ago, see https://www.python.org/downloads/release/python-390/ .

Ooops sorry you are right. I was confusing two things: type hints were added around 3.6 (ish) which was that long ago, but my proposal was actually for 3.9 which has even more goodies.

travisdowns commented 2 months ago

@nyh wrote:

Will "type annotations" help anything?

My biased opinion is that type hints are very useful and low overhead and a net positive for anything which is non-trivial (and in my view addr2line is already well beyond that point). Especially for scripts which have limited test coverage (here, because we don't have any test "binaries" to run on, the testing omits a big part of the interesting work and so coverage is limited), type hints catch a lot of bugs before they go in.

In any case, I did notice that this ship has already sailed as the existing script had at least a couple of type hints already (yes, the change that introduced them was also mine).