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

sqlalchemy 1.4.x: re-optionalize 'greenlet' dependency #6136

Closed
jayaddison opened this issue Mar 25, 2021 · 28 comments
Closed

sqlalchemy 1.4.x: re-optionalize 'greenlet' dependency #6136

jayaddison opened this issue Mar 25, 2021 · 28 comments
Labels
arm issue related to the arm architecture greenlet install question issue where a "fix" on the SQLAlchemy side is unlikely, hence more of a usage question setup issues related to installation and setup use case not really a feature or a bug; can be support for new DB features or user use cases not anticipated

Comments

@jayaddison
Copy link

Is your feature request related to a problem? Please describe.
The greenlet dependency was restored to the default sqlalchemy package dependencies in 1.4, and that's understandable since many users will want and benefit from the async functionality provided by greenlet without having to specify extras (specifically asyncio) in their requirements files.

That said it does make it challenging (impossible?) to use sqlalchemy in pure-python deployment environments currently. If no binary package of greenlet nor a compiler toolchain for greenlet to build with are available at install-time, then sqlalchemy package installation will fail.

Describe the solution you'd like
Suggestions from people more familiar with the Python packaging ecosystem than I am would be appreciated :)

@jayaddison jayaddison added the requires triage New issue that requires categorization label Mar 25, 2021
@zzzeek zzzeek added use case not really a feature or a bug; can be support for new DB features or user use cases not anticipated and removed requires triage New issue that requires categorization labels Mar 26, 2021
@zzzeek
Copy link
Member

zzzeek commented Mar 26, 2021

hi -

I agree it would be nice to not have greenlet as a required dependency, but then we would be flooded with bug reports of people eager to use asyncio and then confused why it doesnt work, or frustrated that it raises errors and that they need to run some separate step for it to work.

What we can do instead is have greenlet only install if the Python environment is not a so-called "pure python" environment. Can you identify what this means exactly?

@zzzeek zzzeek added the question issue where a "fix" on the SQLAlchemy side is unlikely, hence more of a usage question label Mar 26, 2021
@CaselIT
Copy link
Member

CaselIT commented Mar 26, 2021

What we can do instead is have greenlet only install if the Python environment is not a so-called "pure python" environment. Can you identify what this means exactly?

The usual culprit here is alpine linux that's somewhat popular as a docker base image. That cannot use the manylinux wheels that are published on pypi. I'm not sure how we could detect in in the install though

@jayaddison
Copy link
Author

What we can do instead is have greenlet only install if the Python environment is not a so-called "pure python" environment. Can you identify what this means exactly?

The usual culprit here is alpine linux that's somewhat popular as a docker base image.

That's precisely it in this case, yep.

That cannot use the manylinux wheels that are published on pypi. I'm not sure how we could detect in in the install though

Thanks! - that gives me something to search about to see whether I can track this down further (or think of workarounds).

@CaselIT
Copy link
Member

CaselIT commented Mar 26, 2021

Thanks! - that gives me something to search about to see whether I can track this down further (or think of workarounds).

If you could find a way I think we could add it in the install requirements.

As a side note, consider using python-slim as the base image instead of alpine since, depending on the use case, can be noticeably slower https://superuser.com/questions/1219609/why-is-the-alpine-docker-image-over-50-slower-than-the-ubuntu-image

@jayaddison
Copy link
Author

Thank you - I also just discovered https://pythonspeed.com/articles/alpine-docker-python/ while reading about this, and it draws similar conclusions about increased build times (and image sizes!)

@jayaddison
Copy link
Author

It feels like environment markers could be one way to specify this kind of constraint (install greenlet by default ; if x, y, z) although I don't know what kind of conditions we'd want to apply. I also don't initially see any markers relating to the various versions of manylinux.

@CaselIT
Copy link
Member

CaselIT commented Mar 30, 2021

I'm not sure if it's possible with the environment makers.

Testing them all on python:3.9-alpine and python:3.9-slim I get pretty much equal results. Test stript:

import os
import platform
import sys

print(os.name)
print(sys.platform)
print(platform.machine())
print(platform.python_implementation())
print(platform.release())
print(platform.system())
print(platform.version())
print('.'.join(platform.python_version_tuple()[:2]))
print(platform.python_version())
print(sys.implementation.name)

def format_full_version(info):
    version = '{0.major}.{0.minor}.{0.micro}'.format(info)
    kind = info.releaselevel
    if kind != 'final':
        version += kind[0] + str(info.serial)
    return version

if hasattr(sys, 'implementation'):
    implementation_version = format_full_version(sys.implementation.version)
else:
    implementation_version = "0"
print(implementation_version)

On python:3.9-alpine i get

posix
linux
x86_64
CPython
4.19.121-linuxkit
Linux
#1 SMP Thu Jan 21 15:36:34 UTC 2021
3.9
3.9.2
cpython
3.9.2

on python:3.9-slim instead

posix
linux
x86_64
CPython
4.19.121-linuxkit
Linux
#1 SMP Thu Jan 21 15:36:34 UTC 2021
3.9
3.9.2
cpython
3.9.2

I'm not sure I tested them all. Maybe these is some other way of getting that info?

@CaselIT CaselIT added the help wanted Extra attention is needed label Mar 30, 2021
@mgorny
Copy link
Contributor

mgorny commented Jul 25, 2021

Another part of the problem is that greenlet has very limited platform support. For example, it is known not to work at all on hppa or ia64 platforms.

@mgorny
Copy link
Contributor

mgorny commented Jul 25, 2021

I wanted to make the test suite skip tests requiring greenlet when it's not installed but the complexity of wrappers used in the test suite is above my pay grade.

@zzzeek
Copy link
Member

zzzeek commented Jul 26, 2021

this should work already if you run pytest directly. I'm able to run the test suite from a virtualenv that does not have greenlet installed. all greenlet tests are skipped as well. if this is not automatic then we need to see what's happening.

in the tox file it looks like greenlet is a dependency, so we'd need to add a tox option or command of some kind.

however for now, just do "pytest" and it should work

@mgorny
Copy link
Contributor

mgorny commented Jul 26, 2021

It doesn't work — we get around a dozen test errors like the following:

_____________________________________________ TestAsyncAdaptedQueue.test_error_other_loop _____________________________________________
Traceback (most recent call last):
  File "/tmp/sqlalchemy/test/../lib/sqlalchemy/testing/plugin/pytestplugin.py", line 772, in decorate
    asyncio._run_coroutine_function(fn, *args, **kwargs)
  File "/tmp/sqlalchemy/test/../lib/sqlalchemy/testing/asyncio.py", line 33, in _run_coroutine_function
    return _util_async_run_coroutine_function(fn, *args, **kwargs)
  File "/tmp/sqlalchemy/test/../lib/sqlalchemy/util/concurrency.py", line 59, in _util_async_run_coroutine_function
    _not_implemented()
  File "/tmp/sqlalchemy/test/../lib/sqlalchemy/util/concurrency.py", line 36, in _not_implemented
    raise ValueError(
ValueError: the greenlet library is required to use this function.

This seems to be directly caused by the @async_test decorator.

@zzzeek
Copy link
Member

zzzeek commented Jul 26, 2021

im fixing those but can you define "greenlet doesnt work on hppa", meaning it doesnt install? or just doesnt work correctly ?

this cant be done in the tox file very well unless i can get appropriate environment flags: https://www.python.org/dev/peps/pep-0496/

@sqla-tester
Copy link
Collaborator

Mike Bayer referenced this issue:

allow pytest to run if greenlet isnt installed https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2981

@mgorny
Copy link
Contributor

mgorny commented Jul 26, 2021

It doesn't install at all:

src/greenlet/greenlet.c:365:6: error: #error "greenlet needs to be ported to this platform, or taught how to detect your compiler properly."

Overall, greenlet seems to rely on very ugly platform-specific hacks.

@zzzeek
Copy link
Member

zzzeek commented Jul 26, 2021

OK we have to list unsupported platforms in tox.ini. what targets do you need excluded? e.g. uname value or:

python -c "import platform; print(platform.machine())"

@zzzeek
Copy link
Member

zzzeek commented Jul 26, 2021

if you try out the patch at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2981 pytest should run completely, or let me know. working on "tox"

@mgorny
Copy link
Contributor

mgorny commented Jul 26, 2021

On our testing hppa machine, I get:

$ python -c "import platform; print(platform.machine())"
parisc

though I suspect it might depend on the actual CPU. On ia64 it's predictably ia64 ;-).

That said, I'm only the messenger here, I don't do hppa/ia64 myself. But I can test on amd64 without greenlet installed.

@mgorny
Copy link
Contributor

mgorny commented Jul 26, 2021

The patch seems to resolve the test failure I've referenced above. However, there are more failing tests due to missing greenlet:

FAILED test/ext/asyncio/test_engine_py3k.py::TextSyncDBAPI::test_sync_driver_execution[<lambda>0] - ValueError: the greenlet library...
FAILED test/ext/asyncio/test_engine_py3k.py::TextSyncDBAPI::test_sync_driver_execution[<lambda>1] - ValueError: the greenlet library...
FAILED test/ext/asyncio/test_engine_py3k.py::TextSyncDBAPI::test_sync_driver_execution[<lambda>2] - ValueError: the greenlet library...
FAILED test/ext/asyncio/test_engine_py3k.py::TextSyncDBAPI::test_sync_driver_run_sync - ValueError: the greenlet library is required...
FAILED test/engine/test_pool.py::PoolEventsTest::test_checkin_event_gc[True-_exclusions0] - ValueError: the greenlet library is requ...
FAILED test/engine/test_pool.py::QueuePoolTest::test_userspace_disconnectionerror_weakref_finalizer[True-_exclusions0] - ValueError:...

@zzzeek
Copy link
Member

zzzeek commented Aug 9, 2021

so the solution I'm going to put up will be based on the platform_machine limiter. Right now the exclude list will be: "platform_machine not in 'ia64 hppa parisc'" however we can add as many architectures as we want here.

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

allow pytest to run if greenlet isnt installed https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2981

@zzzeek
Copy link
Member

zzzeek commented Aug 11, 2021

this will be merged soon and the current platform exclusion list is:

ia64 hppa parisc

there's likely a lot more we can add here i will ask greenlet devs if there's a better way to get this.

@sqla-tester
Copy link
Collaborator

Mike Bayer has proposed a fix for this issue in the master branch:

limit greenlet dependency against non-supported platforms https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/2981

@zzzeek
Copy link
Member

zzzeek commented Aug 11, 2021

OK, change. new idea, i will make supported platforms default to that which we see already have a binary at https://pypi.org/project/greenlet/#files . anyone else wants asyncio they have to specify the [asyncio] extra.

@jayaddison
Copy link
Author

Thank you @zzzeek!

@che-wish
Copy link

Hi @zzzeek, I would like to report a compatibility issue with poetry

Error when installing sqlalchemy 1.4.23 via poetry:


  AttributeError

  'EmptyConstraint' object has no attribute 'allows'

  at /usr/local/Cellar/poetry/1.1.7/libexec/vendor/lib/python3.9/site-packages/poetry/core/version/markers.py:291 in validate
      287│
      288│         if self._name not in environment:
      289│             return True
      290│
    → 291│         return self._constraint.allows(self._parser(environment[self._name]))
      292│
      293│     def without_extras(self):  # type: () -> MarkerTypes
      294│         return self.exclude("extra")

Reproduce steps:

  • In an empty folder, poetry init to create a empty project skeleton
  • poetry add sqlalchemy==1.4.23 and see the error above
  • poetry add sqlalchemy==1.4.22 and see the packages are successfully installed as below
Package operations: 2 installs, 0 updates, 0 removals

  • Installing greenlet (1.1.1)
  • Installing sqlalchemy (1.4.22)

Based on the error message, my guess is that poetry is not able to handle and platform_machine in "x86_64 X86_64 aarch64 AARCH64 ppc64le PPC64LE amd64 AMD64 win32 WIN32" expression for greenlet package.dependencies. So maybe it's a poetry issue, I'm just posting it here for visibility.

@zzzeek
Copy link
Member

zzzeek commented Aug 18, 2021

Hi @zzzeek, I would like to report a compatibility issue with poetry

Error when installing sqlalchemy 1.4.23 via poetry:


  AttributeError

  'EmptyConstraint' object has no attribute 'allows'

  at /usr/local/Cellar/poetry/1.1.7/libexec/vendor/lib/python3.9/site-packages/poetry/core/version/markers.py:291 in validate
      287│
      288│         if self._name not in environment:
      289│             return True
      290│
    → 291│         return self._constraint.allows(self._parser(environment[self._name]))
      292│
      293│     def without_extras(self):  # type: () -> MarkerTypes
      294│         return self.exclude("extra")

Reproduce steps:

* In an empty folder, `poetry init` to create a empty project skeleton

* `poetry add sqlalchemy==1.4.23` and see the error above

* `poetry add sqlalchemy==1.4.22` and see the packages are successfully installed as below
Package operations: 2 installs, 0 updates, 0 removals

  • Installing greenlet (1.1.1)
  • Installing sqlalchemy (1.4.22)

Based on the error message, my guess is that poetry is not able to handle and platform_machine in "x86_64 X86_64 aarch64 AARCH64 ppc64le PPC64LE amd64 AMD64 win32 WIN32" expression for greenlet package.dependencies. So maybe it's a poetry issue, I'm just posting it here for visibility.

the syntax here is part of https://www.python.org/dev/peps/pep-0508/ so please report that to poetry.

@zzzeek
Copy link
Member

zzzeek commented Aug 18, 2021

they have an issue opened at python-poetry/poetry#4398.

bob-beck pushed a commit to openbsd/ports that referenced this issue Dec 30, 2021
it's more or less a requirement for asyncio, cf
https://docs.sqlalchemy.org/en/14/changelog/migration_14.html#asynchronous-io-support-for-core-and-orm

its been debated to make it optional in
sqlalchemy/sqlalchemy#6136 but my experience
with buildbot was that the daemon didnt start at all if greenlet was not
present, and setup.cfg adds the requirement.
@zzzeek zzzeek added arm issue related to the arm architecture setup issues related to installation and setup and removed help wanted Extra attention is needed labels Nov 16, 2022
@zzzeek
Copy link
Member

zzzeek commented Aug 7, 2023

with asyncio now well established we will consider this for 2.1 in #10197

sqlalchemy-bot pushed a commit that referenced this issue Dec 10, 2023
This is a reopen of #6136 essentially that repaired the
test suite to run without greenlet but now this has regressed.
add a tox target that explicitly uninstalls greenlet, will
add to CI.

This also changes 2.0 in that the full tox target will
omit dbdrivers that require greenlet.

Fixes: #10747
Change-Id: Ia7d786d781e591539a388bfbe17b00a59f0e86d9
sqlalchemy-bot pushed a commit that referenced this issue Dec 10, 2023
This is a reopen of #6136 essentially that repaired the
test suite to run without greenlet but now this has regressed.
add a tox target that explicitly uninstalls greenlet, will
add to CI.

This also changes 2.0 in that the full tox target will
omit dbdrivers that require greenlet.

Fixes: #10747
Change-Id: Ia7d786d781e591539a388bfbe17b00a59f0e86d9
(cherry picked from commit 0bd686df43f572cec658c586082f299ab2cd756f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arm issue related to the arm architecture greenlet install question issue where a "fix" on the SQLAlchemy side is unlikely, hence more of a usage question setup issues related to installation and setup use case not really a feature or a bug; can be support for new DB features or user use cases not anticipated
Projects
None yet
Development

No branches or pull requests

6 participants