cockroachdb / sqlalchemy-cockroachdb

SQLAlchemy adapter for CockroachDB
Apache License 2.0
143 stars 56 forks source link

Remove version pins for SQLA & Alembic #247

Closed gordthompson closed 2 months ago

gordthompson commented 2 months ago

does this workflow no longer work?

It's awkward. While CI is passing here I can no longer run make update-requirements locally because greenlet is pinned at 2.0.2 and tox can't build it on my machine. If I unpin it then 3.0.3 gets installed and that works. I haven't looked into it too closely but it may be because my .venv has Python 3.12.

Also, pinned test-requirements means that we're not testing against the latest versions of SQLAlchemy and Alembic (and dependencies) unless we continually make update-requirements. If I do that then I'd have to submit a PR and bother you each time (like this time) because the master branch is protected so I can't approve my own changes, even if all tests pass.

rafiss commented 2 months ago

Also, pinned test-requirements means that we're not testing against the latest versions of SQLAlchemy and Alembic (and dependencies) unless we continually make update-requirements.

I actually view this as a feature, not a bug. It is desirable to know that re-running the same CI execution is guaranteed to achieve the same results and use the same dependencies. To give a more extreme but analogous example, we would also not want to un-pin the version of python used in CI, since that could cause instability across different test runs. It's better to be explicit about when we change the version of anything (be it python itself or a dependency), that way we can decide when we need to deal with potentially breaking changes instead of that task being forced upon us.

You're right, there certainly is a trade-off here between stability versus toil, but I would like to err on the side of stability.

I created https://github.com/cockroachdb/sqlalchemy-cockroachdb/pull/248 after running make update-requirements myself. This removed the greenlet dependency. Does that resolve the original issue you had?

gordthompson commented 2 months ago

superseded by #248