Skip to content
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.

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

Make Database.logger mutable #616

Open
rausnitz opened this issue Aug 12, 2024 · 1 comment
Open

Make Database.logger mutable #616

rausnitz opened this issue Aug 12, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@rausnitz
Copy link

Is your feature request related to a problem? Please describe.

Database.logger is get-only, but I'd like to modify the Logger metadata and the log handler. My use-case is to track when Database.transaction is called, so I can warn when the original Database is being used before the transaction closure has been run. This is a common footgun in Vapor:

_ = try await req.db.transaction { transactionDB in
    // run a query using transactionDB
    // run a query using req.db
    // run a query using transactionDB
}

Describe the solution you'd like

Make it settable. Application.logger is settable. Request.logger is too. So it seems viable to do this.

Additional context

If the maintainers are interested in this change, I can submit a PR.

@rausnitz rausnitz added the enhancement New feature or request label Aug 12, 2024
@gwynne
Copy link
Member

gwynne commented Nov 5, 2024

Because Database has to be implemented by each individual driver, this is more complicated than it first appears - all four drivers would have to be updated individually with Concurrency-safe setters for their respective loggers, as would various baroque types such as LoggingOverrideDatabase (which, ironically, doesn't work properly anyway). This will be solved in Fluent 5, but I'm not really of a mind to deal with it for Fluent 4 at this point. That being said, if you're up for making the five different PRs (one for each driver plus one for fluent-kit) that would be needed, I'm not dead-set against it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants