Skip to content

Commit

Permalink
Fixes to the Delete PR (#928)
Browse files Browse the repository at this point in the history
* Clean up indexes on files and requests to make delete more efficient

* Don't allow the user lookup in the auth decorator to leave a dangling transaction
We are moving from implicit transactions to more fine-grain control. We don't
want to completely turn off implicit transactions during this migration, so any
database interaction can start the transaction which can spoil things later on.
This decorator was causing a transaction to be started for any user-facing REST endpoint.
Change this so it runs in its own transaction and commits it before entering the
endpoint code.

* Fix last remaining reference to the archived property on requests
  • Loading branch information
BenGalewsky authored Nov 26, 2024
1 parent a42ace9 commit d66f0ca
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 39 deletions.
41 changes: 41 additions & 0 deletions servicex_app/migrations/versions/1.5.6_.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
"""empty message
Revision ID: 1.5.6
Revises: v1_5_5
Create Date: 2024-11-23 17:30:18.079736
"""
from alembic import op
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision = 'v1_5_6'
down_revision = 'v1_5_5'
branch_labels = None
depends_on = None

"""
Clean up indexes and constraints in the database
"""
def upgrade():
op.drop_index('ix_dataset_id', table_name='files')
op.drop_constraint('files_dataset_id_fkey', 'files', type_='foreignkey')
op.create_foreign_key(None, 'files', 'datasets', ['dataset_id'], ['id'])
op.alter_column('requests', 'files',
existing_type=sa.INTEGER(),
nullable=False)
op.drop_index('ix_transform_result_request_id', table_name='transform_result')
op.drop_index('ix_transform_result_transform_status', table_name='transform_result')
op.create_index(op.f('ix_users_sub'), 'users', ['sub'], unique=True)


def downgrade():
op.drop_index(op.f('ix_users_sub'), table_name='users')
op.create_index('ix_transform_result_transform_status', 'transform_result', ['transform_status'], unique=False)
op.create_index('ix_transform_result_request_id', 'transform_result', ['request_id'], unique=False)
op.alter_column('requests', 'files',
existing_type=sa.INTEGER(),
nullable=True)
op.drop_constraint(None, 'files', type_='foreignkey')
op.create_foreign_key('files_dataset_id_fkey', 'files', 'datasets', ['dataset_id'], ['id'], ondelete='CASCADE')
op.create_index('ix_dataset_id', 'files', ['dataset_id'], unique=False)
26 changes: 0 additions & 26 deletions servicex_app/migrations/versions/v1_6_0.py

This file was deleted.

30 changes: 18 additions & 12 deletions servicex_app/servicex_app/decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,14 @@
verify_jwt_in_request)
from flask_jwt_extended.exceptions import NoAuthorizationError

from servicex_app.models import UserModel
from servicex_app.models import UserModel, db


@jwt_required()
def get_jwt_user():
jwt_val = get_jwt_identity()
user = UserModel.find_by_sub(jwt_val)

return user


Expand Down Expand Up @@ -53,17 +54,22 @@ def inner(*args, **kwargs) -> Response:
except NoAuthorizationError as exc:
assert "NoAuthorizationError"
return make_response({'message': str(exc)}, 401)
user = get_jwt_user()
if not user:
msg = 'Not Authorized: No user found matching this API token. ' \
'Your account may have been deleted. ' \
'Please visit the ServiceX website to obtain a new API token.'
return make_response({'message': msg}, 401)
elif user.pending:
msg = 'Not Authorized: Your account is still pending. ' \
'An administrator should approve it shortly. If not, ' \
'please contact the ServiceX admins via email or Slack.'
return make_response({'message': msg}, 401)

# Explicitly start a transaction here to avoid unexpected in_transaction() b
# in the method wrapped by this decorator.
with db.session.begin():
user = get_jwt_user()

if not user:
msg = 'Not Authorized: No user found matching this API token. ' \
'Your account may have been deleted. ' \
'Please visit the ServiceX website to obtain a new API token.'
return make_response({'message': msg}, 401)
elif user.pending:
msg = 'Not Authorized: Your account is still pending. ' \
'An administrator should approve it shortly. If not, ' \
'please contact the ServiceX admins via email or Slack.'
return make_response({'message': msg}, 401)

return fn(*args, **kwargs)

Expand Down
2 changes: 1 addition & 1 deletion servicex_app/servicex_app/web/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
def dashboard(template_name: str, user_specific=False):
args = parser.parse_args()
sort, order = args["sort"], args["order"]
query = TransformRequest.query.filter_by(archived=False)
query = TransformRequest.query.filter_by()

if user_specific:
query = query.filter_by(submitted_by=session["user_id"])
Expand Down

0 comments on commit d66f0ca

Please sign in to comment.