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

Decide supported Python 3.x versions (current code is 3.4+) #25

Closed
Oberon00 opened this issue Jun 24, 2019 · 27 comments
Closed

Decide supported Python 3.x versions (current code is 3.4+) #25

Oberon00 opened this issue Jun 24, 2019 · 27 comments
Assignees
Labels
discussion Issue or PR that needs/is extended discussion. meta Related to repo itself, process, community, ...

Comments

@Oberon00
Copy link
Member

Oberon00 commented Jun 24, 2019

In #14 we decided not to support Python 2. But with the current code base, we do not even support 3.6, even though setup.py claims we support 3.4. While it would be nice to be on 3.7 only, I think we should at the very least support 3.6. Demo:

user@pc:~/opentelemetry-python/opentelemetry-api$ cat /etc/issue.net
Ubuntu 18.04.2 LTS
user@pc:~/opentelemetry-python/opentelemetry-api$ python3 --version # Note that this is the latest Python version in this Ubuntu version
Python 3.6.8
user@pc:~/opentelemetry-python/opentelemetry-api$ python3 -c 'import opentelemetry.trace'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/mnt/c/workspaces/misc/opentelemetry-python/opentelemetry-api/opentelemetry/trace/__init__.py", line 65
    from __future__ import annotations
    ^
SyntaxError: future feature annotations is not defined

This is PEP 563 which requires 3.7.

Type hints in general seem to be supported since 3.5 / PEP 484. But note that instance/class variable type hints would only be available since 3.6 (PEP 526).

@c24t
Copy link
Member

c24t commented Jun 24, 2019

It looks like we've got two options to support pre-3.7 python versions: rearrange the classes in the module or use string-valued annotations. From PEP 484:

Our solution, which isn't particularly elegant, but gets the job done, is to allow using string literals in annotations

The string literal should contain a valid Python expression (i.e., compile(lit, '', 'eval') should be a valid code object) and it should evaluate without errors once the module has been fully loaded. The local and global namespace in which it is evaluated should be the same namespaces in which default arguments to the same function would be evaluated.

@c24t
Copy link
Member

c24t commented Jun 24, 2019

Type hints in general seem to be supported since 3.5 / PEP 484. But note that instance/class variable type hints would only be available since 3.6 (PEP 526).

This seems fine to me since they're optional in any case. Type hints are at least syntactically valid in 3.4.

@c24t
Copy link
Member

c24t commented Jul 3, 2019

From SIG call: we'll support 3.4 and have no specific plan to drop support in the future.

@c24t c24t closed this as completed Jul 3, 2019
@Oberon00
Copy link
Member Author

Oberon00 commented Jul 8, 2019

According to https://www.python.org/dev/peps/pep-0429/ and https://devguide.python.org/devcycle/, Python 3.4 is already EOL since 2019-03-18. Also pip complains: DEPRECATION: Python 3.4 support has been deprecated. pip 19.1 will be the last one supporting it. Please upgrade your Python as Python 3.4 won't be maintained after March 2019 (cf PEP 429).

@Oberon00 Oberon00 reopened this Jul 8, 2019
@c24t
Copy link
Member

c24t commented Jul 8, 2019

Like the 2.7 deprecation, we're likely to have a lot of users that still rely on officially deprecated versions. So we have a tradeoff: do we support more users and eat the higher maintenance cost (restricted feature set, use of backports, required use of old versions of tools like pip) or support fewer users and save ourselves some headaches?

FWIW the 2018 JetBrains python developer survey reports that ~80% of users were on a version later than 3.4 as of fall 2018. Anecdotally, we tend to see large customers on older versions of python. I'd expect these customers to represent a small percentage of developers in the survey but a large percentage of end users.

Another argument for being liberal with our minimum required version is that we want to make it easy for other libraries to depend on OT. Setting a high minimum python version means that we may lose instrumentation for another library that sets a lower minimum version -- even if the project that imports the other library is actually using a compatible version.

@Oberon00
Copy link
Member Author

Oberon00 commented Jul 9, 2019

Python 3.4 and "other Python 3.x version" account for 5% in that survey. But what we could do is create a Python 3.4 build and see how much trouble it causes compared to a 3.5 build and decide then. I also reopened this because I felt like in the meeting we only agreed on supporting definitely nothing older than 3.4 but did not clearly decide on supporting 3.4.

@c24t
Copy link
Member

c24t commented Jul 9, 2019

we only agreed on supporting definitely nothing older than 3.4 but did not clearly decide on supporting 3.4.

That's a good point, we can keep it open until we've got a decision on 3.4 support.

@c24t c24t mentioned this issue Jul 10, 2019
@c24t
Copy link
Member

c24t commented Jul 15, 2019

#46 adds testenvs for 3.4, 3.5, and 3.6, but doesn't (yet) run them during CI.

@Jamim
Copy link
Contributor

Jamim commented Aug 20, 2019

Hi there,

Probably, it's time to drop support for Python 3.4.
How do you think?

@Jamim
Copy link
Contributor

Jamim commented Aug 22, 2019

Probably, it's time to drop support for Python 3.4.
How do you think?

Any thoughts?

@Oberon00
Copy link
Member Author

We should discuss this in today's SIG meeting (8:00 PST, see https://www.google.com/calendar/event?eid=NnNvbTJjYjZjZGdtNmJiMzc0cDNnYjlrNzFpbTJiOW9jOHIzNGI5aGM1Z2owYzlwYzhzbThkYjNjZ18yMDE5MDgyMlQxNTAwMDBaIGdvb2dsZS5jb21fYjc5ZTNlOTBqN2Jic2EybjJwNWFuNWxmNjBAZw)

@Jamim
Copy link
Contributor

Jamim commented Aug 22, 2019

@Oberon00 Am I free to join the SIG meeting using Zoom?

@Oberon00
Copy link
Member Author

Of course! 😃 The SIG meetings are open to all interested parties (see https://github.com/open-telemetry/community#community-meetings). You are very welcome to join!

@carlosalberto
Copy link
Contributor

carlosalberto commented Aug 22, 2019

Things that need or work better with 3.5:

  • typing
  • async def syntax
  • contextvars

@toumorokoshi
Copy link
Member

toumorokoshi commented Aug 22, 2019

Notes from SIG meeting 08/22:

  • python3.5 supports arrow return types
  • python3.5 supports async / def support
  • New Relic has a policy that matches PSF's EOL dates, or two years by default
  • DataDog client seems to support 3.4 and 2.7. Not clear if that's related to PSF's EOL policy.

@reyang
Copy link
Member

reyang commented Aug 22, 2019

Related to #62.

@Oberon00
Copy link
Member Author

Related #101 (if we use the contextvars backport, we might not need our thread local contextvars implementation).

@Oberon00 Oberon00 added discussion Issue or PR that needs/is extended discussion. meta Related to repo itself, process, community, ... labels Sep 24, 2019
@Oberon00
Copy link
Member Author

Python 3.5 has typing.ContextManager[T] (see #198 (comment))

@c24t c24t added this to the Alpha v0.4 milestone Oct 11, 2019
@codeboten
Copy link
Contributor

Important to note that the contextvars backport currently doesn't support asyncio: MagicStack/contextvars#2

@Oberon00 Oberon00 changed the title Decide supported Python 3.x versions (current code is 3.7 only) Decide supported Python 3.x versions (current code is 3.4+) Jan 17, 2020
@ocelotl
Copy link
Contributor

ocelotl commented Jan 17, 2020

Important to note that the contextvars backport currently doesn't support asyncio: MagicStack/contextvars#2

The aiocontextvars package was created as a workaround to this problem. Nevertheless, I have found it to produce wrong results too.

This means that we currently don't have a way to use contextvars when the Pyhton version is less than 3.7, unfortunately. Considering this, I doubt we should include the contextvars backport as it was introduced in #101.

@condorcet
Copy link

This means that we currently don't have a way to use contextvars when the Pyhton version is less than 3.7, unfortunately.

Agree with this. In OT we faced this problem too and one of the solution is patching contextvars with patched aiocontextvars fantix/aiocontextvars#66 Yes, sounds crazy :) In my opinion it's better to support python >= 3.7 and maybe inform users that they could achieve needed functionality with mentioned hacks.

@ocelotl
Copy link
Contributor

ocelotl commented Jan 17, 2020

This means that we currently don't have a way to use contextvars when the Pyhton version is less than 3.7, unfortunately.

Agree with this. In OT we faced this problem too and one of the solution is patching contextvars with patched aiocontextvars fantix/aiocontextvars#66 Yes, sounds crazy :) In my opinion it's better to support python >= 3.7 and maybe inform users that they could achieve needed functionality with mentioned hacks.

But, didn't you find any issues with aiocontextvars, @condorcet? I tried it yesterday and got this error. 🤷‍♂️

@ocelotl
Copy link
Contributor

ocelotl commented Jan 17, 2020

This means that we currently don't have a way to use contextvars when the Pyhton version is less than 3.7, unfortunately.

Agree with this. In OT we faced this problem too and one of the solution is patching contextvars with patched aiocontextvars fantix/aiocontextvars#66 Yes, sounds crazy :) In my opinion it's better to support python >= 3.7 and maybe inform users that they could achieve needed functionality with mentioned hacks.

But, didn't you found any issues with aiocontextvars, @condorcet? I tried it yesterday and got this error.

Thanks to @condorcet, I have been able to close the previous issue I mentioned (thank you, @condorcet!).

Now, we are only facing this other issue for the oldest versions of Python 3.5.

@toumorokoshi toumorokoshi removed this from the Alpha v0.4 milestone Feb 27, 2020
@c24t
Copy link
Member

c24t commented Mar 5, 2020

@toumorokoshi points out that we need to document the quirks around 3.4 support to close this. We'll keep supporting 3.4 for now.

@toumorokoshi toumorokoshi self-assigned this Mar 5, 2020
@codeboten
Copy link
Contributor

FWIW, the number of packages that are dropping Python 3.4 support is growing. For example installing docker-compose in Python 3.4 now requires pinning the version of PyYAML to 5.2. It might be worth revisiting removing support for 3.4 sooner than later.

@codeboten
Copy link
Contributor

Closing for now, will reopen in the future when it's time to drop 3.4

@ffe4
Copy link
Contributor

ffe4 commented Sep 9, 2020

With EOL for Python 3.5 approaching this Sunday (2020-09-13), is it now time to revisit this issue?

srikanthccv pushed a commit to srikanthccv/opentelemetry-python that referenced this issue Nov 1, 2020
* Add package: opentelemetry-types

* Fix review comments

* Fix README
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Issue or PR that needs/is extended discussion. meta Related to repo itself, process, community, ...
Projects
None yet
Development

No branches or pull requests

10 participants