From 5d9b47cb9ae25e5ce8453370c700d16b574ddc0d Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Mon, 26 Sep 2022 21:45:31 +0530 Subject: [PATCH 01/10] Upgrade SQLAlchemy and enable 2.0 warnings --- docker/docker-compose.test.yml | 1 + pytest.ini | 2 +- requirements.txt | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docker/docker-compose.test.yml b/docker/docker-compose.test.yml index ca426ab67..fb042ff60 100644 --- a/docker/docker-compose.test.yml +++ b/docker/docker-compose.test.yml @@ -33,3 +33,4 @@ services: - critiquebrainz_redis environment: PGPASSWORD: critiquebrainz + SQLALCHEMY_WARN_20: 1 diff --git a/pytest.ini b/pytest.ini index d07376c90..4fe5da139 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,3 +1,3 @@ [pytest] testpaths = critiquebrainz -addopts = --cov=critiquebrainz --no-cov-on-fail +addopts = --cov=critiquebrainz --no-cov-on-fail -W always::DeprecationWarning diff --git a/requirements.txt b/requirements.txt index b7f6773de..d9a8689f8 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -brainzutils@git+https://github.com/metabrainz/brainzutils-python.git@v2.6.1 +brainzutils@git+https://github.com/metabrainz/brainzutils-python.git@sqlalchemy-warnings beautifulsoup4==4.8.0 click==8.1.3 Flask-Babel==2.0.0 @@ -30,8 +30,8 @@ sentry-sdk[flask]==1.5.8 redis==4.2.2 msgpack==0.5.6 requests==2.27.1 -SQLAlchemy==1.3.16 -mbdata@git+https://github.com/amCap1712/mbdata.git@upstream-schema-changes +SQLAlchemy==1.4.41 +mbdata@git+https://github.com/amCap1712/mbdata.git@fix-sqlalchemy-warnings sqlalchemy-dst==1.0.1 markupsafe==2.1.1 itsdangerous==2.1.2 From fab7fb0924be7ee679a3e7aafdd6e72467bdd838 Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Mon, 26 Sep 2022 22:21:33 +0530 Subject: [PATCH 02/10] Only pass text-wrapped string to connection.execute Passing a string to Connection.execute() is deprecated and will be removed in version 2.0. Use the text() construct, or the Connection.exec_driver_sql() method to invoke a driver-level SQL string. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) For rationale, see section: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#execute-method-more-strict-execution-options-are-more-prominent --- critiquebrainz/db/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/critiquebrainz/db/__init__.py b/critiquebrainz/db/__init__.py index 3e03eadf5..91390e434 100644 --- a/critiquebrainz/db/__init__.py +++ b/critiquebrainz/db/__init__.py @@ -1,4 +1,4 @@ -from sqlalchemy import create_engine +from sqlalchemy import create_engine, text from sqlalchemy.pool import NullPool # This value must be incremented after schema changes on exported tables! @@ -24,5 +24,5 @@ def init_db_engine(connect_str): def run_sql_script(sql_file_path): with open(sql_file_path) as sql: connection = engine.connect() - connection.execute(sql.read()) + connection.execute(text(sql.read())) connection.close() From 7331b2257b3947f652eaad3d385de3bad26019fd Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Mon, 26 Sep 2022 22:48:29 +0530 Subject: [PATCH 03/10] Remove reliance on implicit autocommit This commit fixes the warning: The current statement is being autocommitted using implicit autocommit, which will be removed in SQLAlchemy 2.0. Use the .begin() method of Engine or Connection in order to use an explicit transaction for DML and DDL statements. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) For rationale, see section: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#library-level-but-not-driver-level-autocommit-removed-from-both-core-and-orm --- critiquebrainz/data/utils.py | 2 +- critiquebrainz/db/__init__.py | 24 +++++++++++++++++++++--- critiquebrainz/db/avg_rating.py | 4 ++-- critiquebrainz/db/comment.py | 10 +++++----- critiquebrainz/db/comment_revision.py | 21 ++++++++++----------- critiquebrainz/db/license.py | 4 ++-- critiquebrainz/db/moderation_log.py | 2 +- critiquebrainz/db/oauth_client.py | 6 +++--- critiquebrainz/db/oauth_grant.py | 4 ++-- critiquebrainz/db/oauth_token.py | 4 ++-- critiquebrainz/db/review.py | 8 ++++---- critiquebrainz/db/revision.py | 2 +- critiquebrainz/db/spam_report.py | 4 ++-- critiquebrainz/db/users.py | 11 +++++------ critiquebrainz/db/vote.py | 4 ++-- 15 files changed, 63 insertions(+), 47 deletions(-) diff --git a/critiquebrainz/data/utils.py b/critiquebrainz/data/utils.py index a462b18a0..4d36a8104 100644 --- a/critiquebrainz/data/utils.py +++ b/critiquebrainz/data/utils.py @@ -14,7 +14,7 @@ def create_all(): - db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_extensions.sql')) + db.run_sql_script_without_transaction(os.path.join(ADMIN_SQL_DIR, 'create_extensions.sql')) db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_types.sql')) db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_tables.sql')) db.run_sql_script(os.path.join(ADMIN_SQL_DIR, 'create_primary_keys.sql')) diff --git a/critiquebrainz/db/__init__.py b/critiquebrainz/db/__init__.py index 91390e434..8c144ccba 100644 --- a/critiquebrainz/db/__init__.py +++ b/critiquebrainz/db/__init__.py @@ -1,3 +1,4 @@ +from flask_debugtoolbar.panels import sqlalchemy from sqlalchemy import create_engine, text from sqlalchemy.pool import NullPool @@ -22,7 +23,24 @@ def init_db_engine(connect_str): def run_sql_script(sql_file_path): - with open(sql_file_path) as sql: - connection = engine.connect() + with open(sql_file_path) as sql, engine.begin() as connection: connection.execute(text(sql.read())) - connection.close() + + +def run_sql_script_without_transaction(sql_file_path): + with open(sql_file_path) as sql, engine.connect() as connection: + connection.connection.set_isolation_level(0) + lines = sql.read().splitlines() + try: + for line in lines: + # TODO: Not a great way of removing comments. The alternative is to catch + # the exception sqlalchemy.exc.ProgrammingError "can't execute an empty query" + if line and not line.startswith("--"): + connection.execute(text(line)) + except sqlalchemy.exc.ProgrammingError as e: + print("Error: {}".format(e)) + return False + finally: + connection.connection.set_isolation_level(1) + connection.close() + return True diff --git a/critiquebrainz/db/avg_rating.py b/critiquebrainz/db/avg_rating.py index 78576fbdf..ccc68605a 100644 --- a/critiquebrainz/db/avg_rating.py +++ b/critiquebrainz/db/avg_rating.py @@ -46,7 +46,7 @@ def update(entity_id, entity_type): delete(entity_id, entity_type) return avg_rating = int(sum / count + 0.5) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO avg_rating(entity_id, entity_type, rating, count) VALUES (:entity_id, :entity_type, :rating, :count) @@ -70,7 +70,7 @@ def delete(entity_id, entity_type): entity_id (uuid): ID of the entity entity_type (str): Type of the entity """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM avg_rating diff --git a/critiquebrainz/db/comment.py b/critiquebrainz/db/comment.py index 58b9ec271..bc4fd12ac 100644 --- a/critiquebrainz/db/comment.py +++ b/critiquebrainz/db/comment.py @@ -36,7 +36,7 @@ def create(*, user_id, text, review_id, is_draft=False): Returns: the newly created comment in dict form """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO comment (user_id, review_id, is_draft) VALUES (:user_id, :review_id, :is_draft) @@ -47,7 +47,7 @@ def create(*, user_id, text, review_id, is_draft=False): 'is_draft': is_draft, }) comment_id = result.fetchone()['id'] - db_comment_revision.create(comment_id, text) + db_comment_revision.create(connection, comment_id, text) return get_by_id(comment_id) @@ -225,7 +225,7 @@ def delete(comment_id): Args: comment_id (uuid): the ID of the comment to be deleted. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM comment @@ -266,7 +266,7 @@ def update(comment_id, *, text=None, is_draft=None, is_hidden=None): setstr = ', '.join(updates) update_data['comment_id'] = comment_id - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE comment SET {setstr} @@ -274,7 +274,7 @@ def update(comment_id, *, text=None, is_draft=None, is_hidden=None): """.format(setstr=setstr)), update_data) if text is not None: - db_comment_revision.create(comment_id, text) + db_comment_revision.create(connection, comment_id, text) def count_comments(*, review_id=None, user_id=None, is_hidden=None, is_draft=None): diff --git a/critiquebrainz/db/comment_revision.py b/critiquebrainz/db/comment_revision.py index 23b5e2597..9098290d9 100644 --- a/critiquebrainz/db/comment_revision.py +++ b/critiquebrainz/db/comment_revision.py @@ -21,7 +21,7 @@ import critiquebrainz.db as db -def create(comment_id, text): +def create(conn, comment_id, text): """ Create a new revision for comment with specified ID. Args: @@ -31,14 +31,13 @@ def create(comment_id, text): Returns: int: the ID of the new revision created. """ - with db.engine.connect() as connection: - result = connection.execute(sqlalchemy.text(""" - INSERT INTO comment_revision (comment_id, text) - VALUES (:comment_id, :text) - RETURNING id - """), { - 'comment_id': comment_id, - 'text': text, - }) + result = conn.execute(sqlalchemy.text(""" + INSERT INTO comment_revision (comment_id, text) + VALUES (:comment_id, :text) + RETURNING id + """), { + 'comment_id': comment_id, + 'text': text, + }) - return result.fetchone()['id'] + return result.fetchone()['id'] diff --git a/critiquebrainz/db/license.py b/critiquebrainz/db/license.py index 91063f2f3..a4b51f9a1 100644 --- a/critiquebrainz/db/license.py +++ b/critiquebrainz/db/license.py @@ -23,7 +23,7 @@ def create(*, id, full_name, info_url=None): "full_name": full_name, "info_url": info_url, } - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO license(id, full_name, info_url) VALUES (:id, :full_name, :info_url) @@ -37,7 +37,7 @@ def delete(*, id): Args: id (str): ID of the license. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM license diff --git a/critiquebrainz/db/moderation_log.py b/critiquebrainz/db/moderation_log.py index e9f2abb58..ed8568502 100644 --- a/critiquebrainz/db/moderation_log.py +++ b/critiquebrainz/db/moderation_log.py @@ -38,7 +38,7 @@ def create(*, admin_id, review_id=None, user_id=None, raise ValueError("No review ID or user ID specified.") if action not in AdminActions.get_all_actions(): raise ValueError("Please specify a valid action.") - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO moderation_log(admin_id, user_id, review_id, action, timestamp, reason) VALUES (:admin_id, :user_id, :review_id, :action, :timestamp, :reason) diff --git a/critiquebrainz/db/oauth_client.py b/critiquebrainz/db/oauth_client.py index a7d0e8f04..9f423b231 100644 --- a/critiquebrainz/db/oauth_client.py +++ b/critiquebrainz/db/oauth_client.py @@ -29,7 +29,7 @@ def create(*, user_id, name, desc, website, redirect_uri): "redirect_uri": redirect_uri, } """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: row = connection.execute(sqlalchemy.text(""" INSERT INTO oauth_client (client_id, client_secret, redirect_uri, user_id, name, "desc", website) @@ -83,7 +83,7 @@ def update(*, client_id, name=None, desc=None, website=None, redirect_uri=None): """.format(update_str=update_str)) if update_str: - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(query, update_data) @@ -93,7 +93,7 @@ def delete(client_id): Args: client_id(str): ID of the OAuth Client. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM oauth_client diff --git a/critiquebrainz/db/oauth_grant.py b/critiquebrainz/db/oauth_grant.py index f48e96a02..3b08b3c61 100644 --- a/critiquebrainz/db/oauth_grant.py +++ b/critiquebrainz/db/oauth_grant.py @@ -35,7 +35,7 @@ def create(*, client_id, scopes, code, expires, redirect_uri, user_id): "user_id": user_id, "scopes": scopes, } - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO oauth_grant(client_id, code, expires, redirect_uri, scopes, user_id) VALUES (:client_id, :code, :expires, :redirect_uri, :scopes, :user_id) @@ -108,7 +108,7 @@ def delete(*, client_id, code): client_id (str): ID of the OAuth Client. code (str): The authorization code returned from authorization request. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM oauth_grant diff --git a/critiquebrainz/db/oauth_token.py b/critiquebrainz/db/oauth_token.py index 2398074d6..7bccffc6a 100644 --- a/critiquebrainz/db/oauth_token.py +++ b/critiquebrainz/db/oauth_token.py @@ -35,7 +35,7 @@ def create(*, client_id, scopes, access_token, refresh_token, expires, user_id): "user_id": user_id, "scopes": scopes, } - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO oauth_token(client_id, access_token, refresh_token, expires, scopes, user_id) VALUES (:client_id, :access_token, :refresh_token, :expires, :scopes, :user_id) @@ -128,7 +128,7 @@ def delete(*, client_id=None, refresh_token=None, user_id=None): filters.append("user_id = :user_id") filter_data["user_id"] = user_id filterstr = " AND ".join(filters) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM oauth_token diff --git a/critiquebrainz/db/review.py b/critiquebrainz/db/review.py index 594f0ef18..f28d65bfa 100644 --- a/critiquebrainz/db/review.py +++ b/critiquebrainz/db/review.py @@ -210,7 +210,7 @@ def set_hidden_state(review_id, *, is_hidden): is_hidden (bool): True if review has to be hidden, False if not. """ review = get_by_id(review_id) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE review SET is_hidden = :is_hidden @@ -290,7 +290,7 @@ def update(review_id, *, drafted, text=None, rating=None, license_id=None, langu WHERE id = :review_id """.format(setstr=setstr)) - with db.engine.connect() as connection: + with db.engine.begin() as connection: if setstr: updated_info["review_id"] = review_id connection.execute(query, updated_info) @@ -363,7 +363,7 @@ def create(*, entity_id, entity_type, user_id, is_draft, text=None, rating=None, else: published_on = datetime.now() - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO review (id, entity_id, entity_type, user_id, edits, is_draft, is_hidden, license_id, language, source, source_url, published_on) VALUES (:id, :entity_id, :entity_type, :user_id, :edits, :is_draft, :is_hidden, :license_id, :language, :source, :source_url, :published_on) @@ -731,7 +731,7 @@ def delete(review_id): review_id: ID of the review to be deleted. """ review = get_by_id(review_id) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM review diff --git a/critiquebrainz/db/revision.py b/critiquebrainz/db/revision.py index bce09ee49..5e0765c94 100644 --- a/critiquebrainz/db/revision.py +++ b/critiquebrainz/db/revision.py @@ -30,7 +30,7 @@ def get(review_id, limit=1, offset=0): "votes_negative": (int), } """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" SELECT id, review_id, diff --git a/critiquebrainz/db/spam_report.py b/critiquebrainz/db/spam_report.py index aa32ce834..1b1b59edb 100644 --- a/critiquebrainz/db/spam_report.py +++ b/critiquebrainz/db/spam_report.py @@ -49,7 +49,7 @@ def archive(user_id, revision_id): user_id(uuid): ID of the reporter. revision_id(uuid): ID of the revision of the reported review. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE spam_report SET is_archived = 'true' @@ -79,7 +79,7 @@ def create(revision_id, user_id, reason): "is_archived": (bool) } """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO spam_report (user_id, reason, revision_id, reported_at, is_archived) VALUES (:user_id, :reason, :revision_id, :reported_at, :is_archived) diff --git a/critiquebrainz/db/users.py b/critiquebrainz/db/users.py index 85c15873c..9afdf6311 100644 --- a/critiquebrainz/db/users.py +++ b/critiquebrainz/db/users.py @@ -1,6 +1,5 @@ import uuid from datetime import datetime -from hashlib import md5 import sqlalchemy @@ -143,7 +142,7 @@ def create(**user_data): if user_data: raise TypeError('Unexpected **user_data: %r' % user_data) - with db.engine.connect() as connection: + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" INSERT INTO "user" (id, display_name, email, created, musicbrainz_id, is_blocked, license_choice, musicbrainz_row_id) @@ -325,7 +324,7 @@ def unblock(user_id): Args: user_id(uuid): ID of user to be unblocked. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE "user" SET is_blocked = 'false' @@ -341,7 +340,7 @@ def block(user_id): Args: user_id(uuid): ID of user to be blocked. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" UPDATE "user" SET is_blocked = 'true' @@ -608,7 +607,7 @@ def update(user_id, user_new_info): """.format(setstr)) if user_new_info: user_new_info["user_id"] = user_id - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(query, user_new_info) @@ -619,7 +618,7 @@ def delete(user_id): Args: user_id(uuid): ID of the user to be deleted. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM "user" diff --git a/critiquebrainz/db/vote.py b/critiquebrainz/db/vote.py index 3f9eaa1b7..0c2b91eb0 100644 --- a/critiquebrainz/db/vote.py +++ b/critiquebrainz/db/vote.py @@ -47,7 +47,7 @@ def submit(user_id, revision_id, vote): revision_id (id): ID of a review revision that the vote is associated with. vote (bool): `False` if it's a negative vote, `True` if positive. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" INSERT INTO vote (user_id, revision_id, vote, rated_at) VALUES (:user_id, :revision_id, :vote, :rated_at) @@ -67,7 +67,7 @@ def delete(user_id, revision_id): user_id (uuid): ID of a user. revision_id (id): ID of a review revision that the vote is associated with. """ - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(""" DELETE FROM vote WHERE user_id = :user_id AND revision_id = :revision_id From 23a294632ebba7c54a4190d4a3698b41e3597615 Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Tue, 27 Sep 2022 00:09:20 +0530 Subject: [PATCH 04/10] Use result.mappings() to access row as dicts Builds on #2119 This PR fixes the warning: Using non-integer/slice indices on Row is deprecated and will be removed in version 2.0; please use row._mapping[], or the mappings() accessor on the Result object. (Background on SQLAlchemy 2.0 at: https://sqlalche.me/e/b8d9) For rationale, see section: https://docs.sqlalchemy.org/en/14/changelog/migration_20.html#result-rows-act-like-named-tuples --- critiquebrainz/db/__init__.py | 2 +- critiquebrainz/db/avg_rating.py | 3 +- critiquebrainz/db/comment.py | 12 +-- critiquebrainz/db/comment_revision.py | 2 +- critiquebrainz/db/license.py | 2 +- critiquebrainz/db/moderation_log.py | 4 +- critiquebrainz/db/oauth_client.py | 6 +- critiquebrainz/db/oauth_grant.py | 12 +-- critiquebrainz/db/oauth_token.py | 6 +- critiquebrainz/db/review.py | 88 +++++++++---------- critiquebrainz/db/revision.py | 2 +- critiquebrainz/db/spam_report.py | 6 +- critiquebrainz/db/statistics.py | 6 +- critiquebrainz/db/users.py | 63 +++++++------ critiquebrainz/db/vote.py | 4 +- .../frontend/external/bookbrainz_db/author.py | 2 +- .../external/bookbrainz_db/edition.py | 2 +- .../external/bookbrainz_db/edition_group.py | 4 +- .../external/bookbrainz_db/identifiers.py | 2 +- .../external/bookbrainz_db/literary_work.py | 4 +- .../external/bookbrainz_db/publisher.py | 2 +- .../external/bookbrainz_db/redirects.py | 2 +- .../external/bookbrainz_db/relationships.py | 2 +- .../frontend/external/bookbrainz_db/series.py | 2 +- 24 files changed, 119 insertions(+), 121 deletions(-) diff --git a/critiquebrainz/db/__init__.py b/critiquebrainz/db/__init__.py index 8c144ccba..30160c7ff 100644 --- a/critiquebrainz/db/__init__.py +++ b/critiquebrainz/db/__init__.py @@ -28,7 +28,7 @@ def run_sql_script(sql_file_path): def run_sql_script_without_transaction(sql_file_path): - with open(sql_file_path) as sql, engine.connect() as connection: + with open(sql_file_path) as sql, engine.begin() as connection: connection.connection.set_isolation_level(0) lines = sql.read().splitlines() try: diff --git a/critiquebrainz/db/avg_rating.py b/critiquebrainz/db/avg_rating.py index ccc68605a..76a9f0634 100644 --- a/critiquebrainz/db/avg_rating.py +++ b/critiquebrainz/db/avg_rating.py @@ -42,6 +42,7 @@ def update(entity_id, entity_type): # Calculate average rating and update it sum, count = row[0], row[1] + if count == 0: delete(entity_id, entity_type) return @@ -111,7 +112,7 @@ def get(entity_id, entity_type): "entity_type": entity_type }) - avg_rating = result.fetchone() + avg_rating = result.mappings().first() if not avg_rating: raise db_exceptions.NoDataFoundException("""No rating for the entity with ID: {id} and Type: {type}""". format(id=entity_id, type=entity_type)) diff --git a/critiquebrainz/db/comment.py b/critiquebrainz/db/comment.py index bc4fd12ac..cd5d2e56a 100644 --- a/critiquebrainz/db/comment.py +++ b/critiquebrainz/db/comment.py @@ -46,7 +46,7 @@ def create(*, user_id, text, review_id, is_draft=False): 'review_id': review_id, 'is_draft': is_draft, }) - comment_id = result.fetchone()['id'] + comment_id = result.fetchone().id db_comment_revision.create(connection, comment_id, text) return get_by_id(comment_id) @@ -100,7 +100,7 @@ def get_by_id(comment_id): 'comment_id': comment_id, }) - comment = result.fetchone() + comment = result.mappings().first() if not comment: raise db_exceptions.NoDataFoundException('Can\'t find comment with ID: {id}'.format(id=comment_id)) @@ -199,7 +199,7 @@ def list_comments(*, review_id=None, user_id=None, limit=20, offset=0, inc_hidde OFFSET :offset """.format(where_clause=filterstr)), query_vals) - rows = [dict(row) for row in result.fetchall()] + rows = [dict(row) for row in result.mappings()] for row in rows: row['last_revision'] = { 'id': row.pop('last_revision_id'), @@ -273,8 +273,8 @@ def update(comment_id, *, text=None, is_draft=None, is_hidden=None): WHERE id = :comment_id """.format(setstr=setstr)), update_data) - if text is not None: - db_comment_revision.create(connection, comment_id, text) + if text is not None: + db_comment_revision.create(connection, comment_id, text) def count_comments(*, review_id=None, user_id=None, is_hidden=None, is_draft=None): @@ -317,4 +317,4 @@ def count_comments(*, review_id=None, user_id=None, is_hidden=None, is_draft=Non FROM comment {where_condition} """.format(where_condition=filterstr)), filter_data) - return result.fetchone()[0] + return result.fetchone().count diff --git a/critiquebrainz/db/comment_revision.py b/critiquebrainz/db/comment_revision.py index 9098290d9..5210790b1 100644 --- a/critiquebrainz/db/comment_revision.py +++ b/critiquebrainz/db/comment_revision.py @@ -40,4 +40,4 @@ def create(conn, comment_id, text): 'text': text, }) - return result.fetchone()['id'] + return result.fetchone().id diff --git a/critiquebrainz/db/license.py b/critiquebrainz/db/license.py index a4b51f9a1..93a49b965 100644 --- a/critiquebrainz/db/license.py +++ b/critiquebrainz/db/license.py @@ -59,7 +59,7 @@ def get_licenses_list(connection): FROM license """) results = connection.execute(query) - return [dict(row) for row in results.fetchall()] + return [dict(row) for row in results.mappings()] def list_licenses(): diff --git a/critiquebrainz/db/moderation_log.py b/critiquebrainz/db/moderation_log.py index ed8568502..17c800de9 100644 --- a/critiquebrainz/db/moderation_log.py +++ b/critiquebrainz/db/moderation_log.py @@ -81,7 +81,7 @@ def list_logs(*, admin_id=None, limit=None, offset=None): """.format(where_clause=where_clause)) with db.engine.connect() as connection: result = connection.execute(query, filter_data) - count = result.fetchone()[0] + count = result.fetchone().count filter_data["limit"] = limit filter_data["offset"] = offset @@ -142,7 +142,7 @@ def list_logs(*, admin_id=None, limit=None, offset=None): with db.engine.connect() as connection: result = connection.execute(query, filter_data) logs = [] - for log in result.fetchall(): + for log in result.mappings(): log = dict(log) log["user"] = { "id": log["user_id"], diff --git a/critiquebrainz/db/oauth_client.py b/critiquebrainz/db/oauth_client.py index 9f423b231..ce122361d 100644 --- a/critiquebrainz/db/oauth_client.py +++ b/critiquebrainz/db/oauth_client.py @@ -45,7 +45,7 @@ def create(*, user_id, name, desc, website, redirect_uri): "website": website, "desc": desc, }) - return dict(row.fetchone()) + return dict(row.mappings().first()) def update(*, client_id, name=None, desc=None, website=None, redirect_uri=None): @@ -134,7 +134,7 @@ def get_client(client_id): """), { "client_id": client_id, }) - row = result.fetchone() + row = result.mappings().first() if not row: raise db_exceptions.NoDataFoundException("Can't find OAuth client with ID: {id}".format(id=client_id)) - return dict(row) + return dict(row) diff --git a/critiquebrainz/db/oauth_grant.py b/critiquebrainz/db/oauth_grant.py index 3b08b3c61..7a9ea9a9a 100644 --- a/critiquebrainz/db/oauth_grant.py +++ b/critiquebrainz/db/oauth_grant.py @@ -41,7 +41,7 @@ def create(*, client_id, scopes, code, expires, redirect_uri, user_id): VALUES (:client_id, :code, :expires, :redirect_uri, :scopes, :user_id) RETURNING id """), grant) - grant["id"] = result.fetchone()[0] + grant["id"] = result.first().id return grant @@ -97,8 +97,8 @@ def list_grants(*, client_id=None, code=None, limit=1, offset=None): LIMIT :limit OFFSET :offset """.format(where_clause=filterstr)), filter_data) - rows = results.fetchall() - return [dict(row) for row in rows] + rows = results.mappings() + return [dict(row) for row in rows] def delete(*, client_id, code): @@ -136,9 +136,9 @@ def get_scopes(grant_id): """), { "grant_id": grant_id, }) - scopes = result.fetchone() + scopes = result.mappings().first() if not scopes: raise db_exceptions.NoDataFoundException("No grant exists with ID: {grant_id}".format(grant_id=grant_id)) - if scopes[0] is None: + if scopes["scopes"] is None: return list() - return scopes[0].split() + return scopes["scopes"].split() diff --git a/critiquebrainz/db/oauth_token.py b/critiquebrainz/db/oauth_token.py index 7bccffc6a..f686d7291 100644 --- a/critiquebrainz/db/oauth_token.py +++ b/critiquebrainz/db/oauth_token.py @@ -41,7 +41,7 @@ def create(*, client_id, scopes, access_token, refresh_token, expires, user_id): VALUES (:client_id, :access_token, :refresh_token, :expires, :scopes, :user_id) RETURNING id """), token) - token["id"] = result.fetchone()[0] + token["id"] = result.first().id return token @@ -107,8 +107,8 @@ def list_tokens(*, client_id=None, refresh_token=None, access_token=None, limit= LIMIT :limit OFFSET :offset """.format(where_clause=filterstr)), filter_data) - rows = results.fetchall() - return [dict(row) for row in rows] + rows = results.mappings() + return [dict(row) for row in rows] def delete(*, client_id=None, refresh_token=None, user_id=None): diff --git a/critiquebrainz/db/review.py b/critiquebrainz/db/review.py index f28d65bfa..1bd5a49f5 100644 --- a/critiquebrainz/db/review.py +++ b/critiquebrainz/db/review.py @@ -165,41 +165,41 @@ def get_by_ids(review_ids: List[uuid.UUID]): "review_ids": tuple(map(str, review_ids)), }) - reviews = result.fetchall() - - results = [] - for review in reviews: - review = dict(review) - review["rating"] = RATING_SCALE_1_5.get(review["rating"]) - review["last_revision"] = { - "id": review.pop("last_revision_id"), - "timestamp": review.pop("timestamp"), - "text": review.get("text"), - "rating": review.get("rating"), - "review_id": review.get("id"), - } - review["user"] = User({ - "id": review["user_id"], - "display_name": review.pop("display_name", None), - "is_blocked": review.pop("is_blocked", False), - "musicbrainz_username": review.pop("musicbrainz_id"), - "user_ref": review.pop("user_ref"), + reviews = result.mappings() + + results = [] + for review in reviews: + review = dict(review) + review["rating"] = RATING_SCALE_1_5.get(review["rating"]) + review["last_revision"] = { + "id": review.pop("last_revision_id"), + "timestamp": review.pop("timestamp"), + "text": review.get("text"), + "rating": review.get("rating"), + "review_id": review.get("id"), + } + review["user"] = User({ + "id": review["user_id"], + "display_name": review.pop("display_name", None), + "is_blocked": review.pop("is_blocked", False), + "musicbrainz_username": review.pop("musicbrainz_id"), + "user_ref": review.pop("user_ref"), "email": review.pop("email"), - "created": review.pop("user_created"), - }) - review["license"] = { - "id": review["license_id"], - "info_url": review["info_url"], - "full_name": review["full_name"], - } - votes = db_revision.votes(review["last_revision"]["id"]) - review["votes"] = { - "positive": votes["positive"], - "negative": votes["negative"], - } - review["popularity"] = review["votes"]["positive"] - review["votes"]["negative"] - results.append(review) - return results + "created": review.pop("user_created"), + }) + review["license"] = { + "id": review["license_id"], + "info_url": review["info_url"], + "full_name": review["full_name"], + } + votes = db_revision.votes(review["last_revision"]["id"]) + review["votes"] = { + "positive": votes["positive"], + "negative": votes["negative"], + } + review["popularity"] = review["votes"]["positive"] - review["votes"]["negative"] + results.append(review) + return results def set_hidden_state(review_id, *, is_hidden): @@ -242,7 +242,7 @@ def get_count(*, is_draft=False, is_hidden=False): "is_drafted": is_draft, "is_hidden": is_hidden, }) - return result.fetchone()[0] + return result.fetchone().count def update(review_id, *, drafted, text=None, rating=None, license_id=None, language=None, is_draft=None): @@ -295,8 +295,10 @@ def update(review_id, *, drafted, text=None, rating=None, license_id=None, langu updated_info["review_id"] = review_id connection.execute(query, updated_info) db_revision.create(connection, review_id, text, rating) - db_revision.update_rating(review_id) + db_revision.update_rating(review_id) + + with db.engine.begin() as connection: result = connection.execute(sqlalchemy.text(""" SELECT review.entity_id FROM review @@ -304,7 +306,7 @@ def update(review_id, *, drafted, text=None, rating=None, license_id=None, langu """), { "review_id": review_id, }) - review = dict(result.fetchone()) + review = dict(result.mappings().first()) invalidate_ws_entity_cache(review["entity_id"]) @@ -562,7 +564,7 @@ def get_reviews_list(connection, *, inc_drafts=False, inc_hidden=False, entity_i filter_data["offset"] = offset results = connection.execute(query, filter_data) - rows = results.fetchall() + rows = results.mappings() rows = [dict(row) for row in rows] # Organise last revision info in reviews @@ -704,9 +706,8 @@ def get_popular_reviews_for_index(): "limit": defined_limit, "last_month": datetime.now() - timedelta(weeks=4) }) - reviews = results.fetchall() + reviews = [dict(review) for review in results.mappings()] - reviews = [dict(review) for review in reviews] if reviews: for review in reviews: review["rating"] = RATING_SCALE_1_5.get(review["rating"]) @@ -757,7 +758,7 @@ def check_review_deleted(review_id) -> bool: "review_id": review_id, }) - return dict(result.fetchone())["exists"] + return result.first().exists def get_distinct_entities(connection): @@ -771,7 +772,7 @@ def get_distinct_entities(connection): """) results = connection.execute(query) - return {row[0] for row in results.fetchall()} + return {row["entity_id"] for row in results.mappings()} def distinct_entities(): @@ -808,5 +809,4 @@ def reviewed_entities(*, entity_ids, entity_type): "entity_type": entity_type, "entity_ids": tuple(entity_ids), }) - reviewed_ids = [str(row[0]) for row in results.fetchall()] - return reviewed_ids + return [str(row["entity_id"]) for row in results.mappings()] diff --git a/critiquebrainz/db/revision.py b/critiquebrainz/db/revision.py index 5e0765c94..4d40cd02b 100644 --- a/critiquebrainz/db/revision.py +++ b/critiquebrainz/db/revision.py @@ -57,7 +57,7 @@ def get(review_id, limit=1, offset=0): "limit": limit }) - rows = result.fetchall() + rows = result.mappings().all() if not rows: raise db_exceptions.NoDataFoundException("Cannot find specified review.") rows = [dict(row) for row in rows] diff --git a/critiquebrainz/db/spam_report.py b/critiquebrainz/db/spam_report.py index 1b1b59edb..82a8cc0f6 100644 --- a/critiquebrainz/db/spam_report.py +++ b/critiquebrainz/db/spam_report.py @@ -38,8 +38,8 @@ def get(user_id, revision_id): "revision_id": revision_id, }) - report = result.fetchone() - return dict(report) if report else None + report = result.mappings().first() + return dict(report) if report else None def archive(user_id, revision_id): @@ -185,7 +185,7 @@ def list_reports(**kwargs): with db.engine.connect() as connection: result = connection.execute(query, filter_data) - spam_reports = result.fetchall() + spam_reports = result.mappings() if spam_reports: spam_reports = [dict(spam_report) for spam_report in spam_reports] for spam_report in spam_reports: diff --git a/critiquebrainz/db/statistics.py b/critiquebrainz/db/statistics.py index 652042ece..931c21adf 100644 --- a/critiquebrainz/db/statistics.py +++ b/critiquebrainz/db/statistics.py @@ -85,7 +85,7 @@ def get_users_with_review_count(from_date=date(1970, 1, 1), to_date=date.today() "to_date": to_date, }) - reviewers = result.fetchall() + reviewers = result.mappings() if not reviewers: raise db_exceptions.NoDataFoundException("Can't get users with review count!") reviewers = [dict(reviewer) for reviewer in reviewers] @@ -126,7 +126,7 @@ def get_users_with_vote_count(from_date=date(1970, 1, 1), to_date=date.today() + "to_date": to_date, }) - voters = result.fetchall() + voters = result.mappings() if not voters: raise db_exceptions.NoDataFoundException("Can't get users with vote count!") voters = [dict(voter) for voter in voters] @@ -172,7 +172,7 @@ def get_users_with_comment_count(from_date=date(1970, 1, 1), to_date=date.today( "to_date": to_date, }) - commenters = result.fetchall() + commenters = result.mappings() if not commenters: raise db_exceptions.NoDataFoundException("Can't get users with comment count!") commenters = [dict(commenter) for commenter in commenters] diff --git a/critiquebrainz/db/users.py b/critiquebrainz/db/users.py index 9afdf6311..1cf463990 100644 --- a/critiquebrainz/db/users.py +++ b/critiquebrainz/db/users.py @@ -41,7 +41,7 @@ def get_many_by_mb_username(usernames): """.format(columns=','.join(USER_GET_COLUMNS))), { 'musicbrainz_usernames': usernames, }) - row = result.fetchall() + row = result.mappings() users = [dict(r) for r in row] return users @@ -81,11 +81,10 @@ def get_user_by_id(connection, user_id): result = connection.execute(query, { "user_id": user_id }) - row = result.fetchone() - if not row: - return None - row = dict(row) - return row + row = result.mappings().first() + if row: + return dict(row) + return None def get_by_id(user_id): @@ -159,7 +158,7 @@ def create(**user_data): "license_choice": license_choice, "musicbrainz_row_id": musicbrainz_row_id, }) - new_id = result.fetchone()[0] + new_id = result.fetchone().id return get_by_id(new_id) @@ -189,11 +188,10 @@ def get_by_mbid(musicbrainz_username): """.format(columns=','.join(USER_GET_COLUMNS))), { "musicbrainz_username": musicbrainz_username, }) - row = result.fetchone() - if not row: - return None - row = dict(row) - return row + row = result.mappings().first() + if row: + return dict(row) + return None def get_or_create(musicbrainz_row_id, musicbrainz_username, new_user_data): @@ -280,7 +278,7 @@ def total_count(): FROM "user" """)) - return result.fetchone()[0] + return result.fetchone().count def list_users(limit=None, offset=0): @@ -313,9 +311,8 @@ def list_users(limit=None, offset=0): "limit": limit, "offset": offset }) - rows = result.fetchall() - rows = [dict(row) for row in rows] - return rows + rows = result.mappings() + return [dict(row) for row in rows] def unblock(user_id): @@ -371,8 +368,8 @@ def has_voted(user_id, review_id): "revision_id": last_revision['id'], "user_id": user_id }) - count = result.fetchone()[0] - return count > 0 + count = result.fetchone().count + return count > 0 def karma(user_id): @@ -448,8 +445,8 @@ def reviews(user_id): "user_id": user_id }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_votes(user_id, from_date=datetime.utcfromtimestamp(0)): @@ -478,8 +475,8 @@ def get_votes(user_id, from_date=datetime.utcfromtimestamp(0)): "from_date": from_date }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_reviews(user_id, from_date=datetime.utcfromtimestamp(0)): @@ -531,8 +528,8 @@ def get_reviews(user_id, from_date=datetime.utcfromtimestamp(0)): "from_date": from_date }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_comments(user_id, from_date=datetime.utcfromtimestamp(0)): @@ -576,8 +573,8 @@ def get_comments(user_id, from_date=datetime.utcfromtimestamp(0)): "from_date": from_date }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def update(user_id, user_new_info): @@ -661,8 +658,8 @@ def clients(user_id): "user_id": user_id }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def tokens(user_id): @@ -702,8 +699,8 @@ def tokens(user_id): "user_id": user_id }) - rows = result.fetchall() - return [dict(row) for row in rows] + rows = result.mappings() + return [dict(row) for row in rows] def get_by_mb_row_id(musicbrainz_row_id, musicbrainz_id=None): @@ -733,7 +730,7 @@ def get_by_mb_row_id(musicbrainz_row_id, musicbrainz_id=None): WHERE musicbrainz_row_id = :musicbrainz_row_id {optional_filter} """.format(columns=','.join(USER_GET_COLUMNS), optional_filter=filter_str)), filter_data) - - if result.rowcount: - return dict(result.fetchone()) + row = result.mappings().first() + if row: + return dict(row) return None diff --git a/critiquebrainz/db/vote.py b/critiquebrainz/db/vote.py index 0c2b91eb0..3e0b92680 100644 --- a/critiquebrainz/db/vote.py +++ b/critiquebrainz/db/vote.py @@ -31,7 +31,7 @@ def get(user_id, revision_id): "user_id": user_id, "revision_id": revision_id, }) - row = result.fetchone() + row = result.mappings().first() if not row: raise db_exceptions.NoDataFoundException("Cannot find specified vote.") return dict(row) @@ -85,4 +85,4 @@ def get_count(): SELECT count(*) FROM vote """)) - return result.fetchone()[0] + return result.fetchone().count diff --git a/critiquebrainz/frontend/external/bookbrainz_db/author.py b/critiquebrainz/frontend/external/bookbrainz_db/author.py index c38ff36d8..fc3afcd47 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/author.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/author.py @@ -102,7 +102,7 @@ def fetch_multiple_authors(bbids: List[str]) -> dict: gender.name """), {'bbids': tuple(bbids)}) - authors = result.fetchall() + authors = result.mappings() results = {} for author in authors: author = dict(author) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/edition.py b/critiquebrainz/frontend/external/bookbrainz_db/edition.py index d04f009d2..e729a1f3e 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/edition.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/edition.py @@ -79,7 +79,7 @@ def fetch_multiple_editions(bbids: List[str]) -> dict: format """), {'bbids': tuple(bbids)}) - editions = result.fetchall() + editions = result.mappings() results = {} for edition in editions: edition = dict(edition) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py b/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py index fe68b1f24..a9aba36be 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/edition_group.py @@ -78,7 +78,7 @@ def fetch_multiple_edition_groups(bbids: List[str]) -> dict: relationship_set_id """), {'bbids': tuple(bbids)}) - edition_groups = result.fetchall() + edition_groups = result.mappings() results = {} for edition_group in edition_groups: edition_group = dict(edition_group) @@ -123,7 +123,7 @@ def fetch_works_for_edition_group(bbid: uuid.UUID): AND rel.type_id = :relationship_type_id """), {'bbid': str(bbid), 'relationship_type_id': EDITION_WORK_CONTAINS_REL_ID}) - works = result.fetchall() + works = result.mappings() work_bbids = [] for work in works: diff --git a/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py b/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py index a88dcc8ae..b44460257 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/identifiers.py @@ -33,7 +33,7 @@ def fetch_bb_external_identifiers(identifier_set_id: int) -> List: LEFT JOIN identifier_type idtype on iden.type_id = idtype.id WHERE idens.set_id = :identifier_set_id """), {'identifier_set_id': identifier_set_id}) - identifiers = result.fetchall() + identifiers = result.mappings() identifiers = [dict(identifier) for identifier in identifiers] identifiers = process_bb_identifiers(identifiers) cache.set(bb_identifiers_key, identifiers, DEFAULT_CACHE_EXPIRATION) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py b/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py index b4cf2ac19..5a3cf10c5 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/literary_work.py @@ -103,7 +103,7 @@ def fetch_multiple_literary_works(bbids: List[str], work_type=None, limit=None, """.format(work_type_filter_string=work_type_filter_string) ), query_params) - literary_works = result.fetchall() + literary_works = result.mappings() results = {} for literary_work in literary_works: literary_work = dict(literary_work) @@ -144,7 +144,7 @@ def fetch_edition_groups_for_works(bbid: str) -> list: AND rel.type_id = :relationship_type_id """), {'bbid': str(bbid), 'relationship_type_id': EDITION_WORK_CONTAINS_REL_ID}) - edition_groups = result.fetchall() + edition_groups = result.mappings() edition_group_bbids = [] for edition_group in edition_groups: diff --git a/critiquebrainz/frontend/external/bookbrainz_db/publisher.py b/critiquebrainz/frontend/external/bookbrainz_db/publisher.py index 84f9b1f66..29589f303 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/publisher.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/publisher.py @@ -70,7 +70,7 @@ def fetch_multiple_publishers(bbids: List[str]) -> dict: AND master = 't' """), {'bbids': tuple(bbids)}) - publishers = result.fetchall() + publishers = result.mappings() results = {} for publisher in publishers: publisher = dict(publisher) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/redirects.py b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py index cbb7b7d88..65c61db79 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/redirects.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/redirects.py @@ -34,7 +34,7 @@ def get_redirected_bbid(bbid: str) -> str | None: FROM redirects """), {'bbid': bbid}) - redirect_bbids = result.fetchall() + redirect_bbids = result.mappings() results = [] for redirect_bbid in redirect_bbids: diff --git a/critiquebrainz/frontend/external/bookbrainz_db/relationships.py b/critiquebrainz/frontend/external/bookbrainz_db/relationships.py index 908fc36b8..9f9595a86 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/relationships.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/relationships.py @@ -42,7 +42,7 @@ def fetch_relationships(relationship_set_id: int, relation_types_id: List) -> Li WHERE rels.set_id = :relationship_set_id AND reltype.id in :relation_types_id """), {'relationship_set_id': relationship_set_id, 'relation_types_id': tuple(relation_types_id)}) - relationships = result.fetchall() + relationships = result.mappings() relationships = [dict(relationship) for relationship in relationships] cache.set(key, relationships, DEFAULT_CACHE_EXPIRATION) diff --git a/critiquebrainz/frontend/external/bookbrainz_db/series.py b/critiquebrainz/frontend/external/bookbrainz_db/series.py index 94e0cc1b4..79806d2a6 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/series.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/series.py @@ -74,7 +74,7 @@ def fetch_multiple_series(bbids: List[str]) -> dict: AND entity_type IS NOT NULL """), {'bbids': tuple(bbids)}) - data = result.fetchall() + data = result.mappings() results = {} for series in data: series = dict(series) From 55fc866ec7fccab8309fba3cea3391e86c424dc8 Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Tue, 27 Sep 2022 15:35:05 +0530 Subject: [PATCH 05/10] Turn sqlalchemy.exc.Base20DeprecationWarning into errors in tests To avoid introducing more warnings between now and a future PR that will switch the future flag to turn these warnings into errors in prod, turn these warning into errors in tests. --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index 4fe5da139..534060909 100644 --- a/pytest.ini +++ b/pytest.ini @@ -1,3 +1,3 @@ [pytest] testpaths = critiquebrainz -addopts = --cov=critiquebrainz --no-cov-on-fail -W always::DeprecationWarning +addopts = --cov=critiquebrainz --no-cov-on-fail -W always::DeprecationWarning -W error::sqlalchemy.exc.Base20DeprecationWarning From 50e05a0c9ff0c5c7d385c34c3010310376485dde Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Fri, 30 Sep 2022 16:41:14 +0530 Subject: [PATCH 06/10] Update BU to v2.7.0 --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index d9a8689f8..4d1d790e7 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -brainzutils@git+https://github.com/metabrainz/brainzutils-python.git@sqlalchemy-warnings +brainzutils@git+https://github.com/metabrainz/brainzutils-python.git@v2.7.0 beautifulsoup4==4.8.0 click==8.1.3 Flask-Babel==2.0.0 From 042938fb3f04dcf1d45f1f11aae51efa44e86361 Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Tue, 25 Oct 2022 20:02:19 +0530 Subject: [PATCH 07/10] Use fetchall to do without indentation --- critiquebrainz/db/review.py | 68 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/critiquebrainz/db/review.py b/critiquebrainz/db/review.py index 1bd5a49f5..26bca0a83 100644 --- a/critiquebrainz/db/review.py +++ b/critiquebrainz/db/review.py @@ -165,41 +165,41 @@ def get_by_ids(review_ids: List[uuid.UUID]): "review_ids": tuple(map(str, review_ids)), }) - reviews = result.mappings() - - results = [] - for review in reviews: - review = dict(review) - review["rating"] = RATING_SCALE_1_5.get(review["rating"]) - review["last_revision"] = { - "id": review.pop("last_revision_id"), - "timestamp": review.pop("timestamp"), - "text": review.get("text"), - "rating": review.get("rating"), - "review_id": review.get("id"), - } - review["user"] = User({ - "id": review["user_id"], - "display_name": review.pop("display_name", None), - "is_blocked": review.pop("is_blocked", False), - "musicbrainz_username": review.pop("musicbrainz_id"), - "user_ref": review.pop("user_ref"), + reviews = result.mappings().fetchall() + + results = [] + for review in reviews: + review = dict(review) + review["rating"] = RATING_SCALE_1_5.get(review["rating"]) + review["last_revision"] = { + "id": review.pop("last_revision_id"), + "timestamp": review.pop("timestamp"), + "text": review.get("text"), + "rating": review.get("rating"), + "review_id": review.get("id"), + } + review["user"] = User({ + "id": review["user_id"], + "display_name": review.pop("display_name", None), + "is_blocked": review.pop("is_blocked", False), + "musicbrainz_username": review.pop("musicbrainz_id"), + "user_ref": review.pop("user_ref"), "email": review.pop("email"), - "created": review.pop("user_created"), - }) - review["license"] = { - "id": review["license_id"], - "info_url": review["info_url"], - "full_name": review["full_name"], - } - votes = db_revision.votes(review["last_revision"]["id"]) - review["votes"] = { - "positive": votes["positive"], - "negative": votes["negative"], - } - review["popularity"] = review["votes"]["positive"] - review["votes"]["negative"] - results.append(review) - return results + "created": review.pop("user_created"), + }) + review["license"] = { + "id": review["license_id"], + "info_url": review["info_url"], + "full_name": review["full_name"], + } + votes = db_revision.votes(review["last_revision"]["id"]) + review["votes"] = { + "positive": votes["positive"], + "negative": votes["negative"], + } + review["popularity"] = review["votes"]["positive"] - review["votes"]["negative"] + results.append(review) + return results def set_hidden_state(review_id, *, is_hidden): From 382013de5103712fd13f90832f5af5838e86f112 Mon Sep 17 00:00:00 2001 From: Kartik Ohri Date: Tue, 25 Oct 2022 20:13:57 +0530 Subject: [PATCH 08/10] Fix remaining usages of deprecated SQLAlchemy stuff --- critiquebrainz/db/users.py | 2 +- .../frontend/external/bookbrainz_db/common_entity.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/critiquebrainz/db/users.py b/critiquebrainz/db/users.py index 1cf463990..f920b0fd0 100644 --- a/critiquebrainz/db/users.py +++ b/critiquebrainz/db/users.py @@ -61,7 +61,7 @@ def get_user_by_ref(user_ref): """.format(columns=','.join(USER_GET_COLUMNS))), { 'user_ref': user_ref, }) - row = result.fetchone() + row = result.mappings().first() if row: return dict(row) return None diff --git a/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py b/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py index 3fb797a3b..a68a8ae00 100644 --- a/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py +++ b/critiquebrainz/frontend/external/bookbrainz_db/common_entity.py @@ -34,7 +34,7 @@ def get_authors_for_artist(artist_mbid) -> List: AND author.data_id IS NOT NULL GROUP BY bbid """), {'artist_mbid': artist_mbid, 'identifier_type': MB_ARTIST_IDENTIFIER_TYPE}) - authors = result.fetchall() + authors = result.mappings() author_bbids = [] for author in authors: @@ -76,7 +76,7 @@ def get_literary_works_for_work(work_mbid) -> List: GROUP BY bbid """), {'work_mbid': work_mbid, 'identifier_type': MB_WORK_IDENTIFIER_TYPE}) - literary_works = result.fetchall() + literary_works = result.mappings() work_bbids = [] for literary_work in literary_works: literary_work = dict(literary_work) From c6716025ec18c7fa9488c3ef31f9877a8d4616d2 Mon Sep 17 00:00:00 2001 From: Alastair Porter Date: Tue, 25 Oct 2022 17:00:38 +0200 Subject: [PATCH 09/10] use .begin() instead of .connect() for UPDATE query --- critiquebrainz/db/users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/critiquebrainz/db/users.py b/critiquebrainz/db/users.py index f920b0fd0..f6e7a9996 100644 --- a/critiquebrainz/db/users.py +++ b/critiquebrainz/db/users.py @@ -253,7 +253,7 @@ def update_username(user, new_musicbrainz_id: str): WHERE id = :cb_id """.format(", ".join(updates)) - with db.engine.connect() as connection: + with db.engine.begin() as connection: connection.execute(sqlalchemy.text(query), { "cb_id": user["id"], "new_musicbrainz_id": new_musicbrainz_id, From 9c3e58dbfe3d08c361640650a49cdbb87571238a Mon Sep 17 00:00:00 2001 From: Alastair Porter Date: Tue, 25 Oct 2022 17:33:02 +0200 Subject: [PATCH 10/10] Set isolation level manually when we don't want an explicit transaction Can't create extensions in a transaction --- critiquebrainz/db/__init__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/critiquebrainz/db/__init__.py b/critiquebrainz/db/__init__.py index 30160c7ff..7aed3d606 100644 --- a/critiquebrainz/db/__init__.py +++ b/critiquebrainz/db/__init__.py @@ -28,8 +28,7 @@ def run_sql_script(sql_file_path): def run_sql_script_without_transaction(sql_file_path): - with open(sql_file_path) as sql, engine.begin() as connection: - connection.connection.set_isolation_level(0) + with open(sql_file_path) as sql, engine.connect().execution_options(isolation_level="AUTOCOMMIT") as connection: lines = sql.read().splitlines() try: for line in lines: @@ -41,6 +40,5 @@ def run_sql_script_without_transaction(sql_file_path): print("Error: {}".format(e)) return False finally: - connection.connection.set_isolation_level(1) connection.close() return True