Skip to content

Bump SQLAlchemy==1.3.24 to support lnt on Windows #33

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

omjavaid
Copy link
Contributor

@omjavaid omjavaid commented Jun 30, 2025

LNT crashes on windows on startup with following error in SQLAlchemy:

File "C:\Users\omair\work\llvm\llvm-lnt\lnt\server\db\v4db.py", line 4, in
import sqlalchemy
File "C:\Users\omair\work\llvm\sandbox\Lib\site-packages\sqlalchemy_init_.py", line 8, in
from . import util as util # noqa
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\omair\work\llvm\sandbox\Lib\site-packages\sqlalchemy\util_init
.py", line 14, in
from ._collections import coerce_generator_arg # noqa
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "C:\Users\omair\work\llvm\sandbox\Lib\site-packages\sqlalchemy\util_collections.py", line 16, in
from .compat import binary_types
File "C:\Users\omair\work\llvm\sandbox\Lib\site-packages\sqlalchemy\util\compat.py", line 331, in
time_func = time.clock
^^^^^^^^^^
AttributeError: module 'time' has no attribute 'clock'

The issue was fixed in SQLAlchemy by sqlalchemy/sqlalchemy@003333f

This patch upgrades SQLAlchemy to 1.3.24 which resolves above error. Version 1.3.24 seemed like a reasonable upgrade containing the required fix as it does not contain any backward incompatible changes in some of the higher version of SQLAlchemy.

LNT crashes on windows on startup with following error in SQLAlchemy:

AttributeError: module 'time' has no attribute 'clock'

This patch upgrades SQLAlchemy to 1.3.24 which resolves above error.
@omjavaid omjavaid requested a review from DavidSpickett June 30, 2025 23:03
Copy link
Contributor

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

That's a big jump to solve a seemingly simple error.

Can you:

  • Include the traceback in the PR description for folks stumbling on this later
  • Cite some change in SQLAlchemy that justifies the version and what got fixed?

@omjavaid
Copy link
Contributor Author

omjavaid commented Jul 1, 2025

That's a big jump to solve a seemingly simple error.

Can you:

  • Include the traceback in the PR description for folks stumbling on this later
  • Cite some change in SQLAlchemy that justifies the version and what got fixed?

I have updated the PR description with more details. Let me know if it explains the choice of upgrading to 1.3.24.

@DavidSpickett
Copy link
Contributor

They don't say what their versioning rules are but considering that https://docs.sqlalchemy.org/en/20/changelog/migration_20.html exists, assuming that breaking changes would mean new major version seems safe.

@DavidSpickett
Copy link
Contributor

That time.clock, https://docs.python.org/3.5/library/time.html#time.clock

Deprecated since version 3.3:

Removed in 3.8: https://docs.python.org/3/whatsnew/3.8.html#api-and-feature-removals

So it's not so much Windows as it is Python >= 3.8.

Copy link
Contributor

@DavidSpickett DavidSpickett left a comment

Choose a reason for hiding this comment

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

LGTM, put whatever information in the PR message that makes sense to you as justification.

Copy link
Contributor

@slydiman slydiman left a comment

Choose a reason for hiding this comment

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

LGTM
I have tested it with python 3.12.3

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.

3 participants