Repository navigation
feat: set user-agent string to distinguish SQLAlchemy requests #115
Description
Activity
- addedtype: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.‘Nice-to-have’ improvement, new feature or different behavior or design.priority: p3Desirable enhancement or fix. May not be included in next release.Desirable enhancement or fix. May not be included in next release.
on Sep 7, 2021 - addedapi: spannerIssues related to the googleapis/python-spanner-sqlalchemy API.Issues related to the googleapis/python-spanner-sqlalchemy API.
on Sep 7, 2021 - addedpriority: p1Important issue which blocks shipping the next release. Will be fixed prior to next release.Important issue which blocks shipping the next release. Will be fixed prior to next release.and removedpriority: p3Desirable enhancement or fix. May not be included in next release.Desirable enhancement or fix. May not be included in next release.
on Sep 7, 2021 Pushed changes for SQLAlchemy: #116
As I see, Django already uses its own user agent (though it's hardcoded): https://github.com/googleapis/python-spanner-django/blob/4643876219e8a54feb94bf14a79f0fe2fbe3971a/django_spanner/base.py#L135
So, the only place left is the DB API itself:
https://github.com/googleapis/python-spanner/blob/48ac924672c8387e3d47da4eace00c3bdbf3baf6/google/cloud/spanner_dbapi/version.py#L19
What name it should have? It's DB API, but it lives ingoogle-cloud-spanner. Is it desired to divide the original client and the DB API user agents?Good question, I think it is good to distinguish between the client library and dbapi. So how about we default dbapi to something like "dbapi/" where version is the Spanner client version as they both get released in the same package?
Reacted by Ilya GurovNot everybody using this package (or
python-spannerfor that matter) uses Django - this library is just the SQLAlchemy dialect for Spanner and has nothing to do with Django.https://github.com/googleapis/python-spanner/blob/48ac924672c8387e3d47da4eace00c3bdbf3baf6/google/cloud/spanner_dbapi/version.py#L19
This also appears to be framework agnostic - shouldn't this be set instead to the library namepython-spannerby default, allowing for an override (which we'd use from this lib)?Looks like I'm just late to the party, based on @IlyaFaer 's MR that's been merged
@IlyaFaer I assume you'll have a PR coming to fix the default in dbapi?
@geudrik I'm a bit torn on whether to default dbapi to have
python-spanner/<version>ordbapi/<version. I think ideally we'd distinguish the Python client from dbapi since they have different use cases. But I understand that it's in the same package so maybe it's confusing to distinguish it.I'm just an internet stranger, so take my input with some salt. It's your repo after all!
If it were me, I'd ensure that the request identified the code actually using the functionality. In this case, dbapi is the lower level and should have a sane default. But since this lib is another layer up (it depends on dbapi), it's the one that's actually invested in the call.
Reacted by skuruppu
Is your feature request related to a problem? Please describe.
The implementation doesn't seem to set a user-agent string to indicate that the requests are coming from SQLAlchemy.
Describe the solution you'd like
Set the user-agent in the dbapi connection.
Describe alternatives you've considered
N/A
Additional context
It seems that dbapi by default sets the
DEFAULT_USER_AGENT = "django_spanner/" + VERSION. This may not be ideal.