Suggestion
Give DatabaseWrapper::acquire_writer() an options object whose notify_observers
field defaults to true, so the observing writer is what a caller gets by default and
opting out is explicit at the call site.
pub struct AcquireWriterOptions {
/// Route the commit through the change observer. Default: `true`.
pub notify_observers: bool,
}
impl Default for AcquireWriterOptions {
fn default() -> Self {
Self { notify_observers: true }
}
}
// unchanged, and still the observing writer
wrapper.acquire_writer().await?
// opting out says so, in the words that describe what happens
wrapper.acquire_writer_with(AcquireWriterOptions { notify_observers: false }).await?
Why
Today the choice is carried by two method names that do not say what separates them:
wrapper.acquire_writer() // routes the commit through the change observer
wrapper.acquire_regular_writer() // silently bypasses it
"Regular" reads as the ordinary one, which is the opposite of how the two should be
chosen. A caller should almost always want the writer that notifies observers. Nothing in
the word "regular" conveys "this write will not notify anyone", so the distinction has to
be carried by documentation instead of by the API.
The consequence is quiet: a write on the bypassing spelling commits normally and never
notifies, so a subscriber that depends on it does not update. There is a tracing::warn!
when observation is enabled, but as the doc comment notes, it compiles out in release
builds because the workspace pins tracing with release_max_level_off.
A named field beats a name for two reasons. notify_observers: false states the effect
rather than implying a category, and the default lives in one place instead of in whichever
method name a caller happens to reach for. It also leaves room for later per-acquisition
options without another method.
One open question on this shape: acquire_writer() and acquire_regular_writer()
currently return different guard types, so a single entry point parameterized at runtime
needs either a unified guard or a separate _with method as sketched above. Which of those
is right is your call.
Alternate suggestion
If an options object is more churn than it is worth, renaming the bypass alone would fix
the naming problem on its own:
| Today |
Renamed |
acquire_writer() |
unchanged |
acquire_writer_with_attached(specs) |
unchanged |
acquire_regular_writer() |
acquire_writer_without_observers() |
That keeps the common path at the shortest name, changes no existing caller, and moves only
the bypass to a name that states what it does.
Notes
We have no call sites of acquire_regular_writer() ourselves, so this is not blocking
anything for us. It came out of a code review where the first question asked was "what is a
regular writer versus just acquire_writer?", which seemed worth passing upstream.
Happy to open a PR for whichever direction looks right.
Suggestion
Give
DatabaseWrapper::acquire_writer()an options object whosenotify_observersfield defaults to
true, so the observing writer is what a caller gets by default andopting out is explicit at the call site.
Why
Today the choice is carried by two method names that do not say what separates them:
"Regular" reads as the ordinary one, which is the opposite of how the two should be
chosen. A caller should almost always want the writer that notifies observers. Nothing in
the word "regular" conveys "this write will not notify anyone", so the distinction has to
be carried by documentation instead of by the API.
The consequence is quiet: a write on the bypassing spelling commits normally and never
notifies, so a subscriber that depends on it does not update. There is a
tracing::warn!when observation is enabled, but as the doc comment notes, it compiles out in release
builds because the workspace pins
tracingwithrelease_max_level_off.A named field beats a name for two reasons.
notify_observers: falsestates the effectrather than implying a category, and the default lives in one place instead of in whichever
method name a caller happens to reach for. It also leaves room for later per-acquisition
options without another method.
One open question on this shape:
acquire_writer()andacquire_regular_writer()currently return different guard types, so a single entry point parameterized at runtime
needs either a unified guard or a separate
_withmethod as sketched above. Which of thoseis right is your call.
Alternate suggestion
If an options object is more churn than it is worth, renaming the bypass alone would fix
the naming problem on its own:
acquire_writer()acquire_writer_with_attached(specs)acquire_regular_writer()acquire_writer_without_observers()That keeps the common path at the shortest name, changes no existing caller, and moves only
the bypass to a name that states what it does.
Notes
We have no call sites of
acquire_regular_writer()ourselves, so this is not blockinganything for us. It came out of a code review where the first question asked was "what is a
regular writer versus just
acquire_writer?", which seemed worth passing upstream.Happy to open a PR for whichever direction looks right.