Properly terminate database connection in omdb #7290
Conversation
Otherwise attempting to do an action that requires a DestructionToken will cause a panic, as it returns from `run_cmd` before calling terminate on the datastore.
dev-tools/omdb/src/bin/omdb/db.rs
Outdated
| pub(crate) async fn run_cmd( | ||
| &self, | ||
| omdb: &Omdb, | ||
| log: &slog::Logger, | ||
| ) -> Result<(), anyhow::Error> { | ||
| let datastore = self.db_url_opts.connect(omdb, log).await?; | ||
| let res = self.run_cmd_inner(&datastore, omdb, log).await; |
There was a problem hiding this comment.
Take it or leave it: we might want to define run_cmd_inner as a function within the scope of run_cmd, as that will prevent it from being called outside run_cmd, even by other code within this module (which making it module-private doesn't achieve). This might be worth doing if we want to be extra certain that nobody accidentally does the wrong thing.
See this playground demonstrating what I mean.
There was a problem hiding this comment.
Alternatively, run_cmd_inner could just be an async block within run_cmd that we immediately .await, as early returns in the async block won't return from the outer function, just the async block (example). That way, we don't need to have a whole other function for it. Up to you if you think that's nicer...
Otherwise attempting to do an action that requires a DestructionToken will cause a panic, as it returns from
run_cmdbefore calling terminate on the datastore.