Skip to content

Conversation

@cjw296
Copy link
Contributor

@cjw296 cjw296 commented Mar 24, 2025

SQLAlchemy currently has an odd requirement around greenlet: greenlet!=0.4.17.
I couldn't spot why this was there from some lightweight annotation digging, but it's a really old version.

Unfortunately, this lets dependency resolvers pick very old version of greenlet that don't work with Python 3.
I discovered this while moving pytest-sqlalchemy to be uv-based and using GitHub Actions for CI, where I've been using uv sync --all-extras --dev --resolution lowest to check that the minimum required versions of dependencies actually work.

Here's the failing CI run: https://github.com/toirl/pytest-sqlalchemy/actions/runs/14029663597/job/39274427927?pr=11

I've picked greenlet 3.0.0 as the minimum version, since it's the first version that appears to have dropped Python 2 support.

This isn't a massive deal, but I think the change does help SQLAlchemy present a more correct view of its dependencies.
It should also be backported to the 1.4 branch, but I don't know what the process is for that.

@cjw296
Copy link
Contributor Author

cjw296 commented Mar 24, 2025

Full output of the failing situation:

Run uv sync --all-extras --dev --resolution lowest
warning: The direct dependency `sqlalchemy-utils` is unpinned. Consider setting a lower bound when using `--resolution lowest` to avoid using outdated versions.
  × Failed to build `greenlet==0.1`
  ├─▶ The build backend returned an error
  ╰─▶ Call to `setuptools.build_meta:__legacy__.build_wheel` failed (exit
      status: 1)

      [stderr]
      Traceback (most recent call last):
        File "<string>", line 14, in <module>
        File
      "/home/runner/work/_temp/setup-uv-cache/builds-v0/.tmpsr7Fjr/lib/python3.11/site-packages/setuptools/build_meta.py",
      line 334, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=[])
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
        File
      "/home/runner/work/_temp/setup-uv-cache/builds-v0/.tmpsr7Fjr/lib/python3.11/site-packages/setuptools/build_meta.py",
      line 304, in _get_build_requires
          self.run_setup()
        File
      "/home/runner/work/_temp/setup-uv-cache/builds-v0/.tmpsr7Fjr/lib/python3.11/site-packages/setuptools/build_meta.py",
      line 522, in run_setup
          super().run_setup(setup_script=setup_script)
        File
      "/home/runner/work/_temp/setup-uv-cache/builds-v0/.tmpsr7Fjr/lib/python3.11/site-packages/setuptools/build_meta.py",
      line 320, in run_setup
          exec(code, locals())
        File "<string>", line 3, in <module>
        File
      "/home/runner/work/_temp/setup-uv-cache/sdists-v[9](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14029663597/job/39274427927?pr=11#step:4:10)/pypi/greenlet/0.1/ueULOOmf3fJ6ZtCM-Vmit/src/ez_setup/__init__.py",
      line 176
          print "Setuptools version",version,"or greater has been installed."
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
      SyntaxError: Missing parentheses in call to 'print'. Did you mean
      print(...)?

      hint: This usually indicates a problem with the package or the build
      environment.
  help: `greenlet` (v0.1) was included because `pytest-sqlalchemy`
        (v0.2.0) depends on `sqlalchemy>=1.4` (v1.4.0) which depends on
        `greenlet<0.4.[17](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14029663597/job/39274427927?pr=11#step:4:18) | >0.4.17`

@cjw296
Copy link
Contributor Author

cjw296 commented Mar 24, 2025

Given that greenlet<3.1 appears to not work on Python 3.13, it may be pragmatic to just go for 3.1 as the minimum:

Run uv sync --all-extras --dev --resolution lowest
Resolved 11 packages in 1.62s
warning: The transitive dependency `iniconfig` is unpinned. Consider setting a lower bound with a constraint when using `--resolution lowest` to avoid using outdated versions.
warning: The transitive dependency `colorama` is unpinned. Consider setting a lower bound with a constraint when using `--resolution lowest` to avoid using outdated versions.
warning: The transitive dependency `packaging` is unpinned. Consider setting a lower bound with a constraint when using `--resolution lowest` to avoid using outdated versions.
   Building iniconfig==0.1
   Building pytest-sqlalchemy @ file:///home/runner/work/pytest-sqlalchemy/pytest-sqlalchemy
   Building greenlet==3.0.0
   Building sqlalchemy==1.4.0
      Built iniconfig==0.1
      Built pytest-sqlalchemy @ file:///home/runner/work/pytest-sqlalchemy/pytest-sqlalchemy
  × Failed to build `greenlet==3.0.0`
  ├─▶ The build backend returned an error
  ╰─▶ Call to `setuptools.build_meta:__legacy__.build_wheel` failed (exit
      status: 1)

      [stdout]
      running bdist_wheel
      running build
      running build_py
...
      building 'greenlet._greenlet' extension
      creating build/temp.linux-x86_64-cpython-313/src/greenlet
      c++ -pthread -fno-strict-overflow -Wsign-compare
      -Wunreachable-code -DNDEBUG -g -O3 -Wall -fPIC -fPIC
      -I/home/runner/work/_temp/setup-uv-cache/builds-v0/.tmpUQFlXY/include
      -I/home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13
      -c src/greenlet/greenlet.cpp -o
      build/temp.linux-x86_64-cpython-313/src/greenlet/greenlet.o

      [stderr]
      /home/runner/work/_temp/setup-uv-cache/builds-v0/.tmpUQFlXY/lib/python3.13/site-packages/setuptools/dist.py:760:
      SetuptoolsDeprecationWarning: License classifiers are deprecated.
      !!

      
      ********************************************************************************
              Please consider removing the following classifiers in favor of a
      SPDX license expression:

              License :: OSI Approved :: MIT License

              See
      https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license
      for details.
      
      ********************************************************************************

      !!
        self._finalize_license_expression()
      warning: no previously-included files found matching 'benchmarks/*.json'
      no previously-included directories found matching 'docs/_build'
      warning: no files found matching '*.py' under directory 'appveyor'
      warning: no previously-included files matching '*.pyc' found anywhere
      in distribution
      warning: no previously-included files matching '*.pyd' found anywhere
      in distribution
      warning: no previously-included files matching '*.so' found anywhere
      in distribution
      warning: no previously-included files matching '.coverage' found
      anywhere in distribution
      In file included from src/greenlet/greenlet_greenlet.hpp:26,
                       from src/greenlet/greenlet_internal.hpp:20,
                       from src/greenlet/greenlet.cpp:19:
      /home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13/internal/pycore_frame.h:8:4:
      error: #error "this header requires Py_BUILD_CORE define"
          8 | #  error "this header requires Py_BUILD_CORE define"
            |    ^~~~~
      In file included from
      /home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13/internal/pycore_frame.h:13:
      /home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13/internal/pycore_code.h:8:4:
      error: #error "this header requires Py_BUILD_CORE define"
          8 | #  error "this header requires Py_BUILD_CORE define"
            |    ^~~~~
      In file included from
      /home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13/internal/pycore_code.h:11:
      /home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13/internal/pycore_lock.h:13:4:
      error: #error "this header requires Py_BUILD_CORE define"
         13 | #  error "this header requires Py_BUILD_CORE define"
            |    ^~~~~
      In file included from
      /home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13/internal/pycore_code.h:12:
      /home/runner/.local/share/uv/python/cpython-3.13.2-linux-x86_64-gnu/include/python3.13/internal/pycore_backoff.h:9:4:
      error: #error "this header requires Py_BUILD_CORE define"
          9 | #  error "this header requires Py_BUILD_CORE define"
            |    ^~~~~
      src/greenlet/greenlet_greenlet.hpp:[10](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:11)4:9: error: ‘_PyCFrame’ does not
      name a type; did you mean ‘_frame’?
        104 |         _PyCFrame* cframe;
            |         ^~~~~~~~~
            |         _frame
      src/greenlet/greenlet_greenlet.hpp:138:29: error: ‘_PyCFrame’ has not
      been declared
        138 |         void set_new_cframe(_PyCFrame& frame) noexcept;
            |                             ^~~~~~~~~
      In file included from src/greenlet/greenlet.cpp:33:
      src/greenlet/TUserGreenlet.cpp: In member function
      ‘virtual greenlet::Greenlet::switchstack_result_t
      greenlet::UserGreenlet::g_initialstub(void*)’:
      src/greenlet/TUserGreenlet.cpp:280:5: error: ‘_PyCFrame’ was not
      declared in this scope; did you mean ‘_frame’?
        280 |     _PyCFrame trace_info;
            |     ^~~~~~~~~
            |     _frame
      src/greenlet/TUserGreenlet.cpp:282:39: error: ‘trace_info’ was not
      declared in this scope
        282 |     this->python_state.set_new_cframe(trace_info);
            |                                       ^~~~~~~~~~
      In file included from src/greenlet/greenlet.cpp:36:
      src/greenlet/TPythonState.cpp: In constructor
      ‘greenlet::PythonState::PythonState()’:
      src/greenlet/TPythonState.cpp:12:6: error: class ‘greenlet::PythonState’
      does not have any field named ‘cframe’
         12 |     ,cframe(nullptr)
            |      ^~~~~~
      src/greenlet/TPythonState.cpp:82:[11](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:12): error: ‘class
      greenlet::PythonState’ has no member named ‘cframe’
         82 |     this->cframe = &PyThreadState_GET()->root_cframe;
            |           ^~~~~~
      src/greenlet/TPythonState.cpp:82:42: error: ‘PyThreadState’ {aka ‘struct
      _ts’} has no member named ‘root_cframe’
         82 |     this->cframe = &PyThreadState_GET()->root_cframe;
            |                                          ^~~~~~~~~~~
      src/greenlet/TPythonState.cpp: In member function ‘void
      greenlet::PythonState::operator<<(const PyThreadState*)’:
      src/greenlet/TPythonState.cpp:[12](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:13)8:11: error: ‘class
      greenlet::PythonState’ has no member named ‘cframe’
        128 |     this->cframe = tstate->cframe;
            |           ^~~~~~
      src/greenlet/TPythonState.cpp:128:28: error: ‘const PyThreadState’ {aka
      ‘const struct _ts’} has no member named ‘cframe’
        128 |     this->cframe = tstate->cframe;
            |                            ^~~~~~
      src/greenlet/TPythonState.cpp:[13](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:14)6:31: error: ‘C_RECURSION_LIMIT’ was not
      declared in this scope; did you mean ‘Py_C_RECURSION_LIMIT’?
        136 |     this->c_recursion_depth = C_RECURSION_LIMIT -
      tstate->c_recursion_remaining;
            |                               ^~~~~~~~~~~~~~~~~
            |                               Py_C_RECURSION_LIMIT
      src/greenlet/TPythonState.cpp:[14](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:15)0:35: error: ‘const PyThreadState’ {aka
      ‘const struct _ts’} has no member named ‘cframe’
        140 |     this->current_frame = tstate->cframe->current_frame;
            |                                   ^~~~~~
      src/greenlet/TPythonState.cpp:[15](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:16)6:42: error: ‘const PyThreadState’ {aka
      ‘const struct _ts’} has no member named ‘trash’
        156 |     this->trash_delete_nesting = tstate->trash.delete_nesting;
            |                                          ^~~~~
      src/greenlet/TPythonState.cpp: In member function ‘void
      greenlet::PythonState::operator>>(PyThreadState*)’:
      src/greenlet/TPythonState.cpp:[17](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:18)5:13: error: ‘PyThreadState’ {aka
      ‘struct _ts’} has no member named ‘cframe’
        175 |     tstate->cframe = this->cframe;
            |             ^~~~~~
      src/greenlet/TPythonState.cpp:175:28: error: ‘class
      greenlet::PythonState’ has no member named ‘cframe’
        175 |     tstate->cframe = this->cframe;
            |                            ^~~~~~
      src/greenlet/TPythonState.cpp:[18](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:19)9:37: error: ‘C_RECURSION_LIMIT’ was not
      declared in this scope; did you mean ‘Py_C_RECURSION_LIMIT’?
        189 |     tstate->c_recursion_remaining = C_RECURSION_LIMIT -
      this->c_recursion_depth;
            |                                     ^~~~~~~~~~~~~~~~~
            |                                     Py_C_RECURSION_LIMIT
      src/greenlet/TPythonState.cpp:[20](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:21)0:13: error: ‘PyThreadState’ {aka
      ‘struct _ts’} has no member named ‘cframe’
        200 |     tstate->cframe->current_frame = this->current_frame;
            |             ^~~~~~
      src/greenlet/TPythonState.cpp:206:13: error: ‘PyThreadState’ {aka
      ‘struct _ts’} has no member named ‘trash’
        206 |     tstate->trash.delete_nesting = this->trash_delete_nesting;
            |             ^~~~~
      src/greenlet/TPythonState.cpp: At global scope:
      src/greenlet/TPythonState.cpp:[26](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:27)6:6: error: variable or field
      ‘set_new_cframe’ declared void
        266 | void PythonState::set_new_cframe(_PyCFrame& frame) noexcept
            |      ^~~~~~~~~~~
      src/greenlet/TPythonState.cpp:266:34: error: ‘_PyCFrame’ was not
      declared in this scope; did you mean ‘_frame’?
        266 | void PythonState::set_new_cframe(_PyCFrame& frame) noexcept
            |                                  ^~~~~~~~~
            |                                  _frame
      src/greenlet/TPythonState.cpp:266:45: error: ‘frame’ was not declared in
      this scope; did you mean ‘_frame’?
        266 | void PythonState::set_new_cframe(_PyCFrame& frame) noexcept
            |                                             ^~~~~
            |                                             _frame
      src/greenlet/greenlet.cpp: In function ‘PyObject*
      mod_get_tstate_trash_delete_nesting(PyObject*)’:
      src/greenlet/greenlet.cpp:1[34](https://github.com/toirl/pytest-sqlalchemy/actions/runs/14030537434/job/39276989709#step:4:35)0:36: error: ‘PyThreadState’ {aka ‘struct
      _ts’} has no member named ‘trash’
       1340 |     return PyLong_FromLong(tstate->trash.delete_nesting);
            |                                    ^~~~~
      error: command '/usr/bin/c++' failed with exit code 1

      hint: This usually indicates a problem with the package or the build
      environment.
  help: `greenlet` (v3.0.0) was included because `pytest-sqlalchemy:dev`
        (v0.2.0) depends on `greenlet`
Error: Process completed with exit code 1.

@CaselIT
Copy link
Member

CaselIT commented Mar 24, 2025

Hi,

Unfortunately, this lets dependency resolvers pick very old version of greenlet that don't work with Python 3.

This seems like a bug in the resolver, not of sqlalchemy. Have you considered reporting this to uv? I doesn't make any sense for it to select a version that doesn't advertise python3 support.

Given that greenlet<3.1 appears to not work on Python 3.13, it may be pragmatic to just go for 3.1 as the minimum:

This again seems uv that's broken. Why does it select an incompatible version?? the resolver has one job and it seems to fail here.

It should also be backported to the 1.4 branch, but I don't know what the process is for that.

I'm not sure if this is possible, as 1.4 still supports python 2 (Don't remember if we had a constraint to install greenlet only on python 3). Also it's unlikely that 1.4 will have further releases

This isn't a massive deal, but I think the change does help SQLAlchemy present a more correct view of its dependencies.

I don't agree that sqlalchemy currently has an incorrect view of the dependencies.

To me the issue appears to be on uv side. Overall uv still seems to still have issues, since more than an user reported broken installs of sqlalchemy when using uv.

That said I'm not attached to that constraint, but I believe you should first report this to uv since these seems uv bugs

Copy link
Member

@CaselIT CaselIT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's first try to solve the uv bug

@cjw296
Copy link
Contributor Author

cjw296 commented Mar 24, 2025

You might be overlooking the core of the issue here.

uv is resolving dependencies correctly, and uv sync --resolution lowest is a useful tool for discovering bugs like this.

To try and explain the problem differently, SQLAlchemy puts no lower bound on its constraint for greenlet, which means that greenlet 0.1, which does not declare that it is incompatible with Python 3, is a valid choice for a dependency resolver to make.

The "bug" here could be argued to be withgreenlet, for not declaring that 0.1 doesn't work with Python 3, but that's a tenuous one given that Python 3 may not have been released when greenlet 0.1 was released (not actually true, since Python 3 was released by 2006, but the state of Python packaging metadata significantly poorer back then.)

There's a potential "feature" here, for uv to exclude releases "so old they can't be expected to work" when using the --resolution lowest mode, but that seems like quite a big and heuristic ask.

My view is that the "bug" here is SQLAlchemy not declaring the minimum version of greenlet with which it will actually work correctly. The reality is that this won't hit many people, since most dependency resolution is "eager" and picks the newest version that will work, but there may well be cases where other requirements that also have greenlet as a dependency resulting in the final resolved version being one that doesn't work with SQLAlchemy.

So, I see this as a small change that improves the correctness of SQLAlchemy's metadata. What are the potential downsides of this change?

@zzzeek
Copy link
Member

zzzeek commented Mar 24, 2025

hi Chris !

So, I see this as a small change that improves the correctness of SQLAlchemy's metadata. What are the potential downsides of this change?

In the general sense, what we're doing is trying very hard not to put ourselves on the hook for supporting things that should be fixed by others. This is both about keeping proper boundaries for the project itself as well as doing a net good for the community in getting other software improved. I'll point to a big giant example that just happened, where python 3.14 decided that it would silently skip over conditionals when evaluating type annotations. And indeed, as I suspected, this really odd behavior was in fact part of the spec they made!. However after much discussion, the cpython folks decided they can in fact fix it. Even though I have a workaround all ready to go for that issue, it's a lot better that they're just going to fix that problem after being pushed.

That's a dramatic example, but the general idea that we try really hard to not resort to workarounds without even suggesting improvements for the upstream systems we depend on.

For the category of non-standard tooling, things like trio/anyio, Poetry, fastapi, when these tools illustrate problems that dont exist at all in the python standard tooling we are understandably even more skeptical overall since to us these appear as tools that duplicate functionality already found elsewhere, with the promise of being "better", but end up introducing weird problems that place a burden on everyone else. We've gotten things fixed in Poetry at least.

as though I weren't droning on long enough, another reason we dig into "small" changes is that we want to see what the implications of the change are and if we are doing this correctly, because if there is a bigger problem to be solved, we'd like to hit it just once and not fix the same problem over and over again for fragments of the software. In this case, the change to only greenlet looks very arbitrary? This command uv --lowest-resolution seems to be introducing the concept that Python requirements have no intrinsic means of lower bounding software versions automatically under any circumstances, which would appear to suggest that any requirement in Python that does not have a lower bound is essentially broken. So why not all the other requirements in pyproject.toml like pymssql, mysql-connector-python, pymysql ? These projects all have old versions that are incompatible with newer pythons as well. did your uv --lowest-resolution test omit these because you aren't working with those databases?

Bigger picture why is it even valid to have a name in a requirements file that has no lower bound? Shouldn't that simply be an error practically by definition? I think if this PR came in with lower bounds for all requirements that would at least make more sense to me?

@CaselIT
Copy link
Member

CaselIT commented Mar 24, 2025

uv is resolving dependencies correctly, and uv sync --resolution lowest is a useful tool for discovering bugs like this.

To try and explain the problem differently, SQLAlchemy puts no lower bound on its constraint for greenlet, which means that greenlet 0.1, which does not declare that it is incompatible with Python 3, is a valid choice for a dependency resolver to make.

I would argue that the resolver should check for a library to declare python 3 compatibility. Is every package bugged because they don't declare python 4 incompatibility? That can't be right

My view is that the "bug" here is SQLAlchemy not declaring the minimum version of greenlet with which it will actually work correctly. The reality is that this won't hit many people, since most dependency resolution is "eager" and picks the newest version that will work, but there may well be cases where other requirements that also have greenlet as a dependency resulting in the final resolved version being one that doesn't work with SQLAlchemy.

Again lets focus on 3.13 behaviour. version 3.0 does not declare support for it, so why uv picks it? Are we saying that to support 3.14 we have to increase every version of every dependency to make sure that they support at least python 3.14? What about currently released version? should we yank them too? This makes absolutely no sense in my view and it is a bug in uv.

So, I see this as a small change that improves the correctness of SQLAlchemy's metadata. What are the potential downsides of this change?

For the change by itself I don't see particular drawbacks, but I don't see why we should do this, when in my opinion the issue is on the tool you are using.
If we were to accept this change with the current behaviour of uv, then 3.14 is released we would need to change again the constraint to ensure that python 3.14 is covered. We are doing the resolved job and from my understanding this is not how you should use version constraints in libraries.

Have you asked to the uv maintainers why that mode of operation behaves in such bugged way? Personally I don't see how it can work in libraries that declare more than 1 or 2 libraries.
For example is pandas bugged too because it allows numpy 1.26 on python 3.13 that does not support it?

Bigger picture why is it even valid to have a name in a requirements file that has no lower bound? Shouldn't that simply be an error practically by definition? I think if this PR came in with lower bounds for all requirements that would at least make more sense to me?

I don't think it would be any better, since when python 3.14 is released we would need to change them all agian. This uv behaviour makes no sense

@zzzeek
Copy link
Member

zzzeek commented Mar 24, 2025

here is the same feature being requested for pip, coming from the openstack world (that I work in): pypa/pip#8085

@zzzeek
Copy link
Member

zzzeek commented Mar 24, 2025

reading the use case there, this tool makes sense for an application that has the final requirements file set up. Note that openstack names all of its dependencies explicitly, even for libraries that are not directly referenced by the application itself.

Chris I think this tool is intended for that approach, where the ultimate downstream application fully names all dependencies and dependencies-of-dependencies. I dont think pointing it at libraries with open ended constraints is going to be a good path.

@pfmoore
Copy link

pfmoore commented Mar 24, 2025

Chris I think this tool is intended for that approach, where the ultimate downstream application fully names all dependencies and dependencies-of-dependencies. I dont think pointing it at libraries with open ended constraints is going to be a good path.

I found this thread from the linked pip feature request. I agree with @zzzeek here - applications can reasonably use something like --resolution lowest to validate that lower bounds specified in the application constraints file are correct, but "testing" dependency specifiers in individual libraries using that feature is neither helpful nor scalable.

For the specific case of pytest-sqlalchemy, you should probably be testing with --resolution lowest-direct, as that will validate your lower bounds, while not putting you in a position where you can't test code under your control without getting other projects to make changes.

@CaselIT
Copy link
Member

CaselIT commented Mar 25, 2025

so to summarise:

  • supporting uv sync --all-extras --dev --resolution lowest is not something we want to do, since it would require manually setting a min version for each dependency for each different python version. This isn't feasible.
  • that said adding a min version to greenlet may make sense. Since sqlalchemy is used by many other project I think 3.0 is a bit too recent, especially for the 2.0 series. Something like >=1.0 may be more appropriate there, since we don't want do cause unneeded issues to users for no particular reason.
  • on 2.1 it's likely fine to use >=3.0, but then again I question why should we do that since >=1.0 works fine in 2.1 too, so it may just be simpler to keep it

thoughs?

@pfmoore
Copy link

pfmoore commented Mar 25, 2025

@CaselIT Who is "we" in your comment? I suggested --resolution lowest-direct for pytest-sqlalchemy, would that not work for you (and if not, why not?) Then all you have to do is fix any constraints on your dependencies, which are under your control.

@zzzeek
Copy link
Member

zzzeek commented Mar 25, 2025

I would vote for >= 1.0 and leave it at that, that's apparently just one non-beta release past the 0.4.17 version.

@CaselIT
Copy link
Member

CaselIT commented Mar 25, 2025

We refers to sqlalchemy devs, that don't maintain pytest-sqlalchemy.

I don't have an opinion regarding the alternative resolution, seems like it would work for pytest-sqlalchemy.
Regarding why not supporting lower from what I gather we would have to list different constraints for each python version, (example 3.13 needs >=3.1 for greenlet), and that not something that I think would add value

@CaselIT CaselIT requested a review from sqla-tester March 25, 2025 19:05
Copy link
Collaborator

@sqla-tester sqla-tester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this is sqla-tester setting up my work on behalf of CaselIT to try to get revision 4bd856b of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester
Copy link
Collaborator

@sqla-tester
Copy link
Collaborator

Federico Caselli (CaselIT) wrote:

Mike do you want a change note?

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5823

@zzzeek
Copy link
Member

zzzeek commented Mar 26, 2025

Federico Caselli (CaselIT) wrote:

Mike do you want a change note?

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/sqlalchemy/+/5823

this can just go in, 1.0 is just one bump past the 0.4.17 version

@sqla-tester
Copy link
Collaborator

@sqla-tester
Copy link
Collaborator

sqlalchemy-bot pushed a commit that referenced this pull request Mar 26, 2025
Add a lower bound constraint on the greenlet version to 1.

Closes: #12459
Pull-request: #12459
Pull-request-sha: 4bd856b

Change-Id: I200861f1706bf261c2e586b96e8cc35dceb7670b
(cherry picked from commit 938e0fe)
@cjw296 cjw296 deleted the patch-1 branch March 27, 2025 08:42
@cjw296
Copy link
Contributor Author

cjw296 commented Mar 27, 2025

Apologies for the delay in replying...

Just a note that greenlet 1.0 doesn't work with Python 3.10 and above, so may not be a good choice.

I agree with @pfmoore 's comment above and said similar on the pip issue: lowest-direct is probably the pragmatic choice to achieve my aims.

I don't agree with @zzzeek that this should only be done for apps; for apps, I think a fully locked down set of versions in a uv.lock file is the only sane way to go, maybe Mike meant frameworks? (like Django, etc, where they're not the final app, but do have a much more constrained set of dependencies to sanely target?) I do think libraries, such as pytest-sqlalchemy and indeed SQLAlchemy should correctly specify the minimum versions they require of their dependencies. In the day and age of supply chain attacks and ever shorter support windows, my personal preference is to have these be more aggressive, but that's definitely "my thing" rather than what I'd suggest others necessarily do.

To Mike's point about "why only greenlet?": because it's the one that caused the CI to blow up ;-) I know the pain of bigger-than-needed changes and really appreciate all the free work OSS maintainers do, so only want to introduce needed changes rather things-that-could-be-changed, unless I want to support that going forwards.

I'm a little confused as to why greenlet is being installed at all, I wonder if uv sync --all-extras is doing all extras even for dependencies? That would certainly feel like a uv bug, let me check...

@CaselIT
Copy link
Member

CaselIT commented Mar 27, 2025

Just a note that greenlet 1.0 doesn't work with Python 3.10 and above, so may not be a good choice.

sure, but that's only a problem if you use that resolver option of uv, that as mentioned is not something we want to support.

SQLAlchemy should correctly specify the minimum versions they require of their dependencies.

we do. sqlalchemy works with greenlet 1.0 (or before that 0.4.17). It's not feasible to list the min version for each python version, since that's by default incomplete, since we cannot list the constraints for newer python version.

I'm a little confused as to why greenlet is being installed at all

it's required in 2.0. will move to an extra in 2.1

@cjw296
Copy link
Contributor Author

cjw296 commented Mar 27, 2025

So, it looks like SQLAlchemy 1.4 always requires greenlet, is that intentional?

I think that's the root cause of the CI failures I'm seeing.

@CaselIT
Copy link
Member

CaselIT commented Mar 27, 2025

2.0 is the same too, so yes, it wasn't left there for 4+ years by mistake

@cjw296
Copy link
Contributor Author

cjw296 commented Mar 27, 2025

Looks like lowest-direct is working.

@CaselIT
Copy link
Member

CaselIT commented Mar 27, 2025

that's great. lower should work also on 2.1 since we removed greenlet as required lib.

@notatallshaw
Copy link

@cjw296 It occurs to me that using lowest with uv sync, which does universal resolution, for libraries like greenlet which does not support being built against newer Python versions with older releases, you likely need to mark greenlet as no-build-package in the uv configuration: https://docs.astral.sh/uv/reference/settings/#no-build-package

If this doesn't make a lock file that avoids building greenlet for Python versions it doesn't support you should discuss with the uv team.

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 this pull request may close these issues.

6 participants