Fix database connection leaks in DEI routes and user CLI commands

[shlokgilda]

Description

• Fixed database connection leaks in DEI API endpoints and user CLI commands. Fixes Database Connection Leaks in API Routes and CLI Commands #3401.
• Three functions were creating database sessions without properly closing them in all code paths (early returns, exceptions)
• Converted dei_track_repo() and dei_report() to use context managers for automatic session cleanup
• Added try-finally blocks to add_user() and reset_password() CLI commands to guarantee session cleanup
• Fixed missing import in dei_track_repo()
• Removed non-functional session.autocommit = True (has no effect in SQLAlchemy 2.x) and added explicit session.commit() to persist BadgingDEI records

Notes for Reviewers

• dei_report() scoping is critical: session must stay open during template rendering (lazy-loads project.repo), then closes before file operations
• All changes follow existing context manager patterns used elsewhere in the codebase (e.g., augur/application/cli/db.py)
• Manually tested augur user add and augur user password_reset commands - sessions properly close in all paths
• DEI endpoints (dei_track_repo, dei_report) should be manually tested (although I don't think these are even used?)

Signed commits

• Yes, I signed my commits.

[MoralCode]

• to persist BadgingDEI records
  DEI endpoints (dei_track_repo, dei_report) should be manually tested (although I don't think these are even used?)

i think that table is unused and i plan to drop it soon lol (just filed #3423)
I suspect you are right about the routes too.

Fixed database connection leaks in DEI API endpoints and user CLI commands

funny you say that. I'm fairly sure that the DatabaseSession object itself is causing leaks, even when used with with based on chatting with other maintainers. Gen AI suggests that this is because the methods often returns raw Result objects due to db calls not using functions like .one() that specify what kind of results they want, meaning connections are held onto and brought outside the with scope.

Overall it seems like you are trying to do several things in this PR that may be better to split into separate, more focused PRs.

[shlokgilda]

Okay, yeah! The "too many things in one PR" sounds like me.

I'll try to be more vigilant of this.

Re DatabaseSession: I didn't check that. If that's the case, that should fix a whole bunch of issues.

[Copilot]

Pull Request Overview

This PR fixes database connection leaks in DEI API endpoints and user CLI commands by ensuring proper session cleanup in all code paths.

• Converted DEI route functions to use context managers for automatic session cleanup
• Added try-finally blocks to user CLI commands to guarantee session cleanup on early returns and exceptions
• Fixed missing import and removed non-functional SQLAlchemy 2.x code

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
augur/application/cli/user.py	Added try-finally blocks to add_user() and reset_password() commands to ensure session cleanup in all code paths (early returns, exceptions)
augur/api/routes/dei.py	Converted dei_track_repo() and dei_report() to use context managers for automatic session cleanup; fixed import statement; replaced non-functional session.autocommit = True with explicit session.commit()

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Variable user_type is not used.

Suggested change

user_type = "admin user" if admin else "user"

[MoralCode]

@shlokgilda are there tests to confirm that the core of this change does in fact leak fewer connections?

[shlokgilda]

Hmm, good question! I don't think so. Might have to figure out a way to calculate the number of connections before/after the fix.

Any suggestions?

[sgoggins]

LGTM