Skip to content

Fix database connection leaks in DEI routes and user CLI commands #3412

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sgoggins merged 1 commit into from
Dec 17, 2025
+108 −98
Merged

Fix database connection leaks in DEI routes and user CLI commands #3412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 81 additions & 74 deletions augur/api/routes/dei.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from augur.application.db.models import ClientApplication, CollectionStatus, Repo, RepoGroup, BadgingDEI
from augur.application.db.session import DatabaseSession

from augur.tasks.util.collection_util import CollectionRequest,AugurTaskRoutine, get_enabled_phase_names_from_config, core_task_success_util
from augur.tasks.util.collection_util import CollectionRequest,AugurTaskRoutine, get_enabled_phase_names_from_config_session, core_task_success_util
from augur.tasks.start_tasks import prelim_phase, primary_repo_collect_phase
from augur.tasks.github.util.util import get_repo_weight_by_issue

Expand All @@ -33,70 +33,71 @@ def dei_track_repo(application: ClientApplication):

if not (dei_id and level and repo_url):
return jsonify({"status": "Missing argument"}), 400

repo_url = repo_url.lower()

session = DatabaseSession(logger, engine=current_app.engine)
session.autocommit = True
repo: Repo = session.query(Repo).filter(Repo.repo_git==repo_url).first()
if repo:
# Making the assumption that only new repos will be added with this endpoint
return jsonify({"status": "Repo already exists"})

frontend_repo_group: RepoGroup = session.query(RepoGroup).filter(RepoGroup.rg_name == FRONTEND_REPO_GROUP_NAME).first()
repo_id = Repo.insert_github_repo(session, repo_url, frontend_repo_group.repo_group_id, "API.DEI", repo_type="")
if not repo_id:
return jsonify({"status": "Error adding repo"})

repo = Repo.get_by_id(session, repo_id)
repo_git = repo.repo_git
pr_issue_count = get_repo_weight_by_issue(logger, repo_git)

record = {
"repo_id": repo_id,
"issue_pr_sum": pr_issue_count,
"core_weight": -9223372036854775808,
"secondary_weight": -9223372036854775808,
"ml_weight": -9223372036854775808
}

collection_status_unique = ["repo_id"]
session.insert_data(record, CollectionStatus, collection_status_unique, on_conflict_update=False)

record = {
"badging_id": dei_id,
"level": level,
"repo_id": repo_id
}

enabled_phase_names = get_enabled_phase_names_from_config_session(session, logger)

#Primary collection hook.
primary_enabled_phases = []

#Primary jobs
if prelim_phase.__name__ in enabled_phase_names:
primary_enabled_phases.append(prelim_phase)

primary_enabled_phases.append(primary_repo_collect_phase)

#task success is scheduled no matter what the config says.
def core_task_success_util_gen(repo_git):
return core_task_success_util.si(repo_git)

primary_enabled_phases.append(core_task_success_util_gen)

record = BadgingDEI(**record)
session.add(record)

deiHook = CollectionRequest("core",primary_enabled_phases)
deiHook.repo_list = [repo_url]

singleRoutine = AugurTaskRoutine(logger, session,[deiHook])
singleRoutine.start_data_collection()
#start_block_of_repos(logger, session, [repo_url], primary_enabled_phases, "new")

session.close()

# Use context manager to ensure proper session cleanup
with DatabaseSession(logger, engine=current_app.engine) as session:
repo: Repo = session.query(Repo).filter(Repo.repo_git==repo_url).first()
if repo:
# Making the assumption that only new repos will be added with this endpoint
return jsonify({"status": "Repo already exists"})

frontend_repo_group: RepoGroup = session.query(RepoGroup).filter(RepoGroup.rg_name == FRONTEND_REPO_GROUP_NAME).first()
repo_id = Repo.insert_github_repo(session, repo_url, frontend_repo_group.repo_group_id, "API.DEI", repo_type="")
if not repo_id:
return jsonify({"status": "Error adding repo"})

repo = Repo.get_by_id(session, repo_id)
repo_git = repo.repo_git
pr_issue_count = get_repo_weight_by_issue(logger, repo_git)

record = {
"repo_id": repo_id,
"issue_pr_sum": pr_issue_count,
"core_weight": -9223372036854775808,
"secondary_weight": -9223372036854775808,
"ml_weight": -9223372036854775808
}

collection_status_unique = ["repo_id"]
session.insert_data(record, CollectionStatus, collection_status_unique, on_conflict_update=False)

record = {
"badging_id": dei_id,
"level": level,
"repo_id": repo_id
}

enabled_phase_names = get_enabled_phase_names_from_config_session(session, logger)

# Primary collection hook.
primary_enabled_phases = []

# Primary jobs
if prelim_phase.__name__ in enabled_phase_names:
primary_enabled_phases.append(prelim_phase)

primary_enabled_phases.append(primary_repo_collect_phase)

#task success is scheduled no matter what the config says.
def core_task_success_util_gen(repo_git):
return core_task_success_util.si(repo_git)

primary_enabled_phases.append(core_task_success_util_gen)

record = BadgingDEI(**record)
session.add(record)

# Explicitly commit the session to persist BadgingDEI record
session.commit()

deiHook = CollectionRequest("core",primary_enabled_phases)
deiHook.repo_list = [repo_url]

singleRoutine = AugurTaskRoutine(logger, session,[deiHook])
singleRoutine.start_data_collection()
#start_block_of_repos(logger, session, [repo_url], primary_enabled_phases, "new")

return jsonify({"status": "Success"})

Expand All @@ -108,25 +109,31 @@ def dei_report(application: ClientApplication):

if not dei_id:
return jsonify({"status": "Missing argument"}), 400

session = DatabaseSession(logger, engine=current_app.engine)

project: BadgingDEI = session.query(BadgingDEI).filter(BadgingDEI.badging_id==dei_id).first()
# Use context manager but scope it carefully to cover lazy-loading
with DatabaseSession(logger, engine=current_app.engine) as session:
project: BadgingDEI = session.query(BadgingDEI).filter(BadgingDEI.badging_id==dei_id).first()

if not project:
return jsonify({"status": "Invalid ID"})

# Render template while session is still open (accesses project.repo via lazy-loading)
md = render_template("dei-badging-report.j2", project=project)

if not project:
return jsonify({"status": "Invalid ID"})
md = render_template("dei-badging-report.j2", project=project)
# Store project.id before session closes
project_id = project.id

# Session is now closed - proceed with file operations (no database access needed)
cachePath = Path.cwd() / "augur" / "static" / "cache"

source = cachePath / f"{project.id}_badging_report.md"
report = cachePath / f"{project.id}_badging_report.pdf"
source = cachePath / f"{project_id}_badging_report.md"
report = cachePath / f"{project_id}_badging_report.pdf"
source.write_text(md)

command = f"mdpdf -o {str(report.resolve())} {str(source.resolve())}"
converter = subprocess.Popen(command.split())
converter.wait()

# TODO what goes in the report?

return send_file(report.resolve())
51 changes: 27 additions & 24 deletions augur/application/cli/user.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
Add Admin command: augur user add <username> <email> <firstname> <lastname> --admin
'''

import os

Check warning on line 8 in augur/application/cli/user.py

View workflow job for this annotation

GitHub Actions / runner / pylint 

[pylint] reported by reviewdog 🐶
W0611: Unused import os (unused-import)

Raw Output:
augur/application/cli/user.py:8:0: W0611: Unused import os (unused-import)
import click
import logging
from augur.application.db.models import User
Expand Down Expand Up @@ -38,41 +38,44 @@
"""Add a new user to the database with email address = EMAIL."""

session = Session()
try:
if session.query(User).filter(User.login_name == username).first() is not None:
return click.echo("username already taken")

if session.query(User).filter(User.login_name == username).first() is not None:
return click.echo("username already taken")
if session.query(User).filter(User.email == email).first() is not None:
return click.echo("email already signed-up")

if session.query(User).filter(User.email == email).first() is not None:
return click.echo("email already signed-up")

user = session.query(User).filter(User.login_name == username).first()
if not user:
password = User.compute_hashsed_password(password)
new_user = User(login_name=username, login_hashword=password, email=email, text_phone=phone_number, first_name=firstname, last_name=lastname, admin=admin, tool_source="User CLI", tool_version=None, data_source="CLI")
session.add(new_user)
session.commit()
user_type = "admin user" if admin else "user"
message = f"Successfully added new: {username}"
click.secho(message, bold=True)
user = session.query(User).filter(User.login_name == username).first()
if not user:
password = User.compute_hashsed_password(password)
new_user = User(login_name=username, login_hashword=password, email=email, text_phone=phone_number, first_name=firstname, last_name=lastname, admin=admin, tool_source="User CLI", tool_version=None, data_source="CLI")
session.add(new_user)
session.commit()
user_type = "admin user" if admin else "user"
Copy link

Copilot AI Nov 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable user_type is not used.

Suggested change
user_type = "admin user" if admin else "user"

Copilot uses AI. Check for mistakes.
message = f"Successfully added new: {username}"
click.secho(message, bold=True)

return 0
finally:
session.close()
engine.dispose()

return 0

@cli.command('password_reset', short_help="Reset a user's password")
@click.argument("username")
@click.password_option(help="New password")
def reset_password(username, password):
session = Session()
try:
user = session.query(User).filter(User.login_name == username).first()

user = session.query(User).filter(User.login_name == username).first()
if not user:
return click.echo("invalid username")

if not user:
return click.echo("invalid username")

password = User.compute_hashsed_password(password)
user.login_hashword = password
session.commit()
password = User.compute_hashsed_password(password)
user.login_hashword = password
session.commit()

return click.echo("Password updated")
return click.echo("Password updated")
finally:
session.close()
engine.dispose()
Loading