This is a reasonable refactoring pattern. Making db.session an explicit parameter instead of relying on the implicit global db.session has some clear benefits:

Explicitness — callers and readers can immediately see which functions touch the database, improving readability and traceability.
Testability — passing session as a parameter makes it straightforward to inject a mock or test session without monkeypatching globals.
Safety — as you noted, it's a safe, non-behavioral change since db.session is still the same object being passed; the difference is only in how it's threaded through.

A few considerations worth noting:

Incremental adoption — since the codebase is large, doing this incrementally (as in your example PR) is practical, but it may create inconsistency in the interim where some functions accept session and others still use the global. A linting rule or type annotation convention could help track progress.
Type errors — your example PR already surfaced some type-checking issues (e.g., missing session arguments in ParagraphIndexProcessor methods), which validates the point that the refactor makes implicit dependencies visible and catches missing plumbing at compile time.
API surface — for deeply nested call chains, threading session through many layers can get verbose. Some projects address this with a context-local pattern or a unit-of-work abstraction, but explicit parameter passing is the simplest and most transparent approach.

Overall, it's a solid incremental improvement to code quality. The main trade-off is verbosity vs. explicitness, and for a production codebase like Dify, explicitness generally wins.

To reply, just mention @dosu.

Docs are dead. Just use Dosu.

Uh oh!

make db.session pass from parameter. #37403

Description

Metadata

Metadata

Assignees

Labels

Type

Fields

Projects

Milestone

Relationships

Development

Issue actions