Skip to content

Commit

Permalink
Fix SQLAlchemyDriver warning (#1096)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulineribeyre committed May 11, 2023
1 parent 46f9f9d commit d786669
Show file tree
Hide file tree
Showing 10 changed files with 56 additions and 50 deletions.
10 changes: 5 additions & 5 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@
"filename": "fence/resources/google/utils.py",
"hashed_secret": "1348b145fa1a555461c1b790a2f66614781091e9",
"is_verified": false,
"line_number": 132
"line_number": 133
}
],
"fence/utils.py": [
Expand All @@ -198,7 +198,7 @@
"filename": "fence/utils.py",
"hashed_secret": "8318df9ecda039deac9868adf1944a29a95c7114",
"is_verified": false,
"line_number": 129
"line_number": 128
}
],
"migrations/versions/a04a70296688_non_unique_client_name.py": [
Expand All @@ -216,7 +216,7 @@
"filename": "migrations/versions/e4c7b0ab68d3_create_tables.py",
"hashed_secret": "adb1fcd33b07abf9b6a064745759accea5cb341f",
"is_verified": false,
"line_number": 22
"line_number": 21
}
],
"migrations/versions/ea7e1b843f82_optional_client_redirect_uri.py": [
Expand Down Expand Up @@ -355,7 +355,7 @@
"filename": "tests/scripting/test_fence-create.py",
"hashed_secret": "e5e9fa1ba31ecd1ae84f75caaa474f3a663f05f4",
"is_verified": false,
"line_number": 301
"line_number": 300
}
],
"tests/test-fence-config.yaml": [
Expand All @@ -368,5 +368,5 @@
}
]
},
"generated_at": "2023-02-22T17:22:09Z"
"generated_at": "2023-05-10T22:20:13Z"
}
10 changes: 2 additions & 8 deletions fence/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
from cdislogging import get_logger
from gen3authz.client.arborist.client import ArboristClient
from flask_wtf.csrf import validate_csrf
from userdatamodel.driver import SQLAlchemyDriver
from werkzeug.middleware.dispatcher import DispatcherMiddleware
from azure.storage.blob import BlobServiceClient
from azure.core.exceptions import ResourceNotFoundError
Expand Down Expand Up @@ -52,7 +51,7 @@
from fence.resources.storage import StorageManager
from fence.resources.user.user_session import UserSessionInterface
from fence.error_handler import get_error_response
from fence.utils import random_str
from fence.utils import get_SQLAlchemyDriver
import fence.blueprints.admin
import fence.blueprints.data
import fence.blueprints.login
Expand Down Expand Up @@ -107,12 +106,7 @@ def app_init(

def app_sessions(app):
app.url_map.strict_slashes = False

# override userdatamodel's `setup_db` function which creates tables
# and runs database migrations, because Alembic handles that now.
# TODO move userdatamodel code to Fence and remove dependencies to it
SQLAlchemyDriver.setup_db = lambda _: None
app.db = SQLAlchemyDriver(config["DB"])
app.db = get_SQLAlchemyDriver(config["DB"])

# app.db.Session is from SQLAlchemyDriver and uses
# SQLAlchemy's sessionmaker. Using scoped_session here ensures
Expand Down
4 changes: 2 additions & 2 deletions fence/resources/google/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
get_valid_service_account_id_for_user,
)

from userdatamodel.driver import SQLAlchemyDriver
from userdatamodel.user import GoogleProxyGroup, User, AccessPrivilege

from fence.auth import current_token
Expand All @@ -33,6 +32,7 @@
)
from fence.resources.google import STORAGE_ACCESS_PROVIDER_NAME
from fence.errors import NotSupported, NotFound
from fence.utils import get_SQLAlchemyDriver

from cdislogging import get_logger

Expand Down Expand Up @@ -967,6 +967,6 @@ def is_google_managed_service_account(service_account_email):

def get_db_session(db=None):
if db:
return SQLAlchemyDriver(db).Session()
return get_SQLAlchemyDriver(db).Session()
else:
return current_app.scoped_session()
50 changes: 27 additions & 23 deletions fence/scripting/fence_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from cirrus.config import config as cirrus_config
from cdislogging import get_logger
from sqlalchemy import func
from userdatamodel.driver import SQLAlchemyDriver
from userdatamodel.models import (
AccessPrivilege,
Bucket,
Expand Down Expand Up @@ -57,7 +56,12 @@
from fence.scripting.google_monitor import email_users_without_access, validation_check
from fence.config import config
from fence.sync.sync_users import UserSyncer
from fence.utils import create_client, get_valid_expiration, generate_client_credentials
from fence.utils import (
create_client,
get_valid_expiration,
generate_client_credentials,
get_SQLAlchemyDriver,
)

from gen3authz.client.arborist.client import ArboristClient

Expand All @@ -66,7 +70,7 @@

def list_client_action(db):
try:
driver = SQLAlchemyDriver(db)
driver = get_SQLAlchemyDriver(db)
with driver.session as s:
for row in s.query(Client).all():
pprint.pprint(row.__dict__)
Expand All @@ -89,7 +93,7 @@ def modify_client_action(
append=False,
expires_in=None,
):
driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as s:
client_name = client
clients = s.query(Client).filter(Client.name == client_name).all()
Expand Down Expand Up @@ -169,7 +173,7 @@ def delete_client_action(DB, client_name):
pass

try:
driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as current_session:
if (
not current_session.query(Client)
Expand Down Expand Up @@ -212,7 +216,7 @@ def split_uris(uris):
return uris.split("\n")

now = datetime.now().timestamp()
driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
expired_messages = ["Some expired OIDC clients have been deleted!"]
with driver.session as current_session:
clients = (
Expand Down Expand Up @@ -291,7 +295,7 @@ def rotate_client_action(DB, client_name, expires_in=None):
Returns:
This functions does not return anything, but it prints the new set of credentials.
"""
driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as s:
client = s.query(Client).filter(Client.name == client_name).first()
if not client:
Expand Down Expand Up @@ -510,7 +514,7 @@ def create_sample_data(DB, yaml_file_path):
with open(yaml_file_path, "r") as f:
data = safe_load(f)

db = SQLAlchemyDriver(DB)
db = get_SQLAlchemyDriver(DB)
with db.session as s:
create_cloud_providers(s, data)
create_projects(s, data)
Expand Down Expand Up @@ -705,7 +709,7 @@ def google_init(db):
def remove_expired_google_service_account_keys(db):
cirrus_config.update(**config["CIRRUS_CFG"])

db = SQLAlchemyDriver(db)
db = get_SQLAlchemyDriver(db)
with db.session as current_session:
client_service_accounts = current_session.query(
GoogleServiceAccount, Client
Expand Down Expand Up @@ -776,7 +780,7 @@ def remove_expired_google_service_account_keys(db):
def remove_expired_google_accounts_from_proxy_groups(db):
cirrus_config.update(**config["CIRRUS_CFG"])

db = SQLAlchemyDriver(db)
db = get_SQLAlchemyDriver(db)
with db.session as current_session:
current_time = int(time.time())
logger.info("Current time: {}".format(current_time))
Expand Down Expand Up @@ -828,7 +832,7 @@ def remove_expired_google_accounts_from_proxy_groups(db):


def delete_users(DB, usernames):
driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as session:
# NOTE that calling ``.delete()`` on the query itself will not follow
# cascade deletion rules set up in any relationships.
Expand All @@ -850,7 +854,7 @@ def cleanup_expired_ga4gh_information(DB):
IMPORTANT NOTE: This DOES NOT actually remove authorization, it assumes that the
same expiration was set and honored in the authorization system.
"""
driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as session:
current_time = int(time.time())

Expand Down Expand Up @@ -892,7 +896,7 @@ def delete_expired_google_access(DB):
"""
cirrus_config.update(**config["CIRRUS_CFG"])

driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as session:
current_time = int(time.time())

Expand Down Expand Up @@ -947,7 +951,7 @@ def delete_expired_service_accounts(DB):
"""
cirrus_config.update(**config["CIRRUS_CFG"])

driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as session:
current_time = int(time.time())
records_to_delete = (
Expand Down Expand Up @@ -992,7 +996,7 @@ def verify_bucket_access_group(DB):
"""
cirrus_config.update(**config["CIRRUS_CFG"])

driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
with driver.session as session:
access_groups = session.query(GoogleBucketAccessGroup).all()
with GoogleCloudManager() as manager:
Expand Down Expand Up @@ -1145,7 +1149,7 @@ def create_access_token(self):
Return:
JWTResult: result containing the encoded token and claims
"""
driver = SQLAlchemyDriver(self.db)
driver = get_SQLAlchemyDriver(self.db)
with driver.session as current_session:
user = query_for_user(session=current_session, username=self.username)

Expand All @@ -1169,7 +1173,7 @@ def create_refresh_token(self):
Return:
JWTResult: the refresh token result
"""
driver = SQLAlchemyDriver(self.db)
driver = get_SQLAlchemyDriver(self.db)
with driver.session as current_session:
user = query_for_user(session=current_session, username=self.username)

Expand Down Expand Up @@ -1211,7 +1215,7 @@ def link_bucket_to_project(db, bucket_id, bucket_provider, project_auth_id):
bucket_provider (str): CloudProvider.name for the bucket
project_auth_id (str): Project.auth_id to link to bucket
"""
driver = SQLAlchemyDriver(db)
driver = get_SQLAlchemyDriver(db)
with driver.session as current_session:
cloud_provider = (
current_session.query(CloudProvider).filter_by(name=bucket_provider).first()
Expand Down Expand Up @@ -1352,7 +1356,7 @@ def create_or_update_google_bucket(
# default to read access
allowed_privileges = allowed_privileges or ["read", "write"]

driver = SQLAlchemyDriver(db)
driver = get_SQLAlchemyDriver(db)
with driver.session as current_session:
# use storage creds to create bucket
# (default creds don't have permission)
Expand Down Expand Up @@ -1600,7 +1604,7 @@ def link_external_bucket(db, name):

google_project_id = cirrus_config.GOOGLE_PROJECT_ID

db = SQLAlchemyDriver(db)
db = get_SQLAlchemyDriver(db)
with db.session as current_session:
google_cloud_provider = _get_or_create_google_provider(current_session)

Expand Down Expand Up @@ -1691,7 +1695,7 @@ def force_update_google_link(DB, username, google_email, expires_in=None):
"""
cirrus_config.update(**config["CIRRUS_CFG"])

db = SQLAlchemyDriver(DB)
db = get_SQLAlchemyDriver(DB)
with db.session as session:
user_account = query_for_user(session=session, username=username)

Expand Down Expand Up @@ -1762,7 +1766,7 @@ def google_list_authz_groups(db):
db (string): database instance
"""
driver = SQLAlchemyDriver(db)
driver = get_SQLAlchemyDriver(db)

with driver.session as db_session:
google_authz = (
Expand Down Expand Up @@ -1798,7 +1802,7 @@ def access_token_polling_job(
thread_pool_size (int): number of Docker container CPU used for jwt verifcation
buffer_size (int): max size of queue
"""
driver = SQLAlchemyDriver(db)
driver = get_SQLAlchemyDriver(db)
job = Visa_Token_Update(
chunk_size=int(chunk_size) if chunk_size else None,
concurrency=int(concurrency) if concurrency else None,
Expand Down
4 changes: 2 additions & 2 deletions fence/sync/sync_users.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from paramiko.proxy import ProxyCommand
from sqlalchemy.exc import IntegrityError
from sqlalchemy import func
from userdatamodel.driver import SQLAlchemyDriver

from fence.config import config
from fence.models import (
Expand All @@ -42,6 +41,7 @@
from fence.resources.google.access_utils import bulk_update_google_groups
from fence.sync import utils
from fence.sync.passport_sync.ras_sync import RASVisa
from fence.utils import get_SQLAlchemyDriver


def _format_policy_id(path, privilege):
Expand Down Expand Up @@ -336,7 +336,7 @@ def __init__(
self.dbGaP = dbGaP
self.parse_consent_code = dbGaP[0].get("parse_consent_code", True)
self.session = db_session
self.driver = SQLAlchemyDriver(DB)
self.driver = get_SQLAlchemyDriver(DB)
self.project_mapping = project_mapping or {}
self._projects = dict()
self._created_roles = set()
Expand Down
13 changes: 11 additions & 2 deletions fence/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

from cdislogging import get_logger
import flask
from userdatamodel.driver import SQLAlchemyDriver
from werkzeug.datastructures import ImmutableMultiDict

from fence.models import Client, User, query_for_user
Expand Down Expand Up @@ -75,7 +74,7 @@ def create_client(
client_id, client_secret, hashed_secret = generate_client_credentials(confidential)
if arborist is not None:
arborist.create_client(client_id, policies)
driver = SQLAlchemyDriver(DB)
driver = get_SQLAlchemyDriver(DB)
auth_method = "client_secret_basic" if confidential else "none"

allowed_scopes = allowed_scopes or config["CLIENT_ALLOWED_SCOPES"]
Expand Down Expand Up @@ -417,6 +416,16 @@ def get_from_cache(item_id, memory_cache, db_cache_table, db_cache_table_id_fiel
return rv


def get_SQLAlchemyDriver(db_conn_url):
from userdatamodel.driver import SQLAlchemyDriver

# override userdatamodel's `setup_db` function which creates tables
# and runs database migrations, because Alembic handles that now.
# TODO move userdatamodel code to Fence and remove dependencies to it
SQLAlchemyDriver.setup_db = lambda _: None
return SQLAlchemyDriver(db_conn_url)


# Default settings to control usage of backoff library.
DEFAULT_BACKOFF_SETTINGS = {
"on_backoff": log_backoff_retry,
Expand Down
5 changes: 2 additions & 3 deletions migrations/versions/e4c7b0ab68d3_create_tables.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql

from userdatamodel.driver import SQLAlchemyDriver

from bin.old_migration_script import migrate
from fence.utils import get_SQLAlchemyDriver

# revision identifiers, used by Alembic.
revision = "e4c7b0ab68d3"
Expand Down Expand Up @@ -45,7 +44,7 @@ def upgrade():
logger.info(
"Found existing tables: this is not a new instance of Fence. Running the old migration script... Note that future migrations will be run using Alembic."
)
driver = SQLAlchemyDriver(context.config.get_main_option("sqlalchemy.url"))
driver = get_SQLAlchemyDriver(context.config.get_main_option("sqlalchemy.url"))
migrate(driver)
return
else:
Expand Down
1 change: 0 additions & 1 deletion tests/dbgap_sync/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
import pytest
from userdatamodel import Base
from userdatamodel.models import *
from userdatamodel.driver import SQLAlchemyDriver
from gen3authz.client.arborist.client import ArboristClient

from fence.config import config
Expand Down

0 comments on commit d786669

Please sign in to comment.