fix: run compliance tests with nox #26
Conversation
google-cla
bot
added
the
cla: yes
| @@ -41,7 +41,7 @@ | |||
| description=description, | |||
| entry_points={ | |||
| "sqlalchemy.dialects": [ | |||
| "spanner = google.cloud.sqlalchemy_spanner:SpannerDialect" | |||
| "spanner.spanner = google.cloud.sqlalchemy_spanner:SpannerDialect" | |||
There was a problem hiding this comment.
It'll break the package. This identifier is used to register dialect in Python modules system. With this change we'll have to connect like:
engine = create_engine(
"spanner.spanner:///projects/project-id/instances/instance-id/databases/database-id"
)And I don't think it'll work at all.
There was a problem hiding this comment.
I think it's working fine as you can see in SQLAlchemy spanner dialect test.
The url is spanner+spanner:///projects/appdev-soda-spanner-staging/instances/sqlalchemy-dialect-test/databases/compliance-test, which is perfectly working now.
spanner.spanner is used only to fetch the Dialect class. New version of sqlalchemy used by default spanner+spanner and replace + to . to fetch the entry-point or dialect class.
There was a problem hiding this comment.
The other way to overcome the failure is to override the method which added or forcefully converted spanner to spanner+spanner.
The method is generate_driver_url and path is sqlalchemy/testing/provision.py
There was a problem hiding this comment.
I've checked the tests locally, there is no failure at all. These changes are not needed.
There was a problem hiding this comment.
For now compliance tests are failing in you latest opened PR
https://github.com/cloudspannerecosystem/python-spanner-sqlalchemy/pull/27/checks?check_run_id=2119935389
Cloud you help me to solve that issue?
There was a problem hiding this comment.
Currently I am getting the same error as @HemangChothani which is also showing up on the Github Action. I'm not sure why it didn't show up before given that it does seem to be a issue with noxfile.py.
Pinning to an older version of sqlalchemy does not fix the issue on my end.
@IlyaFaer How are you running the compliance tests locally? Are you using the nox command? If not, please post the commands that you are running here so we can update the noxfile to match.
There was a problem hiding this comment.
@larkee In setup.py file I replace dependencies = ["sqlalchemy>=1.1.13"] to dependencies = ["sqlalchemy>=1.1.13, <=1.3.23"] and it works for me.
The reason of failing the command is they have implemented new method generate_driver_url() and path is sqlalchemy/testing/provision.py to generate the url for engine. In that method they replace spanner to spanner+spanner and going to find entry point of the library, in older version there was a spanner as a drive-name so they found perfectly, but in the new version they replace + with . and going to find entry point with spanner.spanner but we have spanner in setup file so it returns None that's why it's failing.
There was a problem hiding this comment.
@larkee, I'm executing tests locally just as usual, the only difference - I don't install packages on every nox session run, I'm using the local ones:
But if I'll uncomment these installing lines in nox session function, everything will still work.
As a simple select-script (executed without nox) is working fine with any version of SQLAlchemy, we can say registering the dialect should not to be changed (that would be strange, if SQLAlchemy team would change dialect registering mechanism in 1.4 version - it would break all the dialects). But nox session is failing, because registering mechanism is not able to find the Spanner dialect class. That makes me think the problem is in installing spanner-sqlalchemy package by nox.
There was a problem hiding this comment.
@IlyaFaer I have tried with manually installed spanner-sqlalchemy and SQLAlchemy and run pytest command manually (WIthout nox) and still sqlalchemy raising the same error. I think they have changed few things in testing procedure.
There was a problem hiding this comment.
I can confirm that updating setup.py with:
dependencies = ["sqlalchemy>=1.1.13, <=1.3.23", "google-cloud-spanner>=3.0.0"]
fixes the issue. The update to entry point is unneeded.
Two additional notes:
- The
compliance_testsession does not need to explicitly installsqlalchemyorgoogle-cloud-spannerbecause they are automatically install bysession.install("-e", ".")which installsspanner-sqlalchemyusingsetup.py create_test_databaseuses the project set by eitherGOOGLE_CLOUD_PROJECTorPROJECT_IDor defaults toemulator-test-projectbut the connection string defined insetup.cfgusing the prod project.
|
Closing this PR in favor of #31 |
Fixes #25