-
Notifications
You must be signed in to change notification settings - Fork 21
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
refactor: drop Transactional and ConnectionOrTransaction #1069
Conversation
modules/analysis/src/endpoints.rs
Outdated
user: UserInformation, | ||
authorizer: web::Data<Authorizer>, | ||
_: Require<ReadSbom>, | ||
) -> actix_web::Result<impl Responder> { | ||
authorizer.require(&user, Permission::ReadSbom)?; | ||
Ok(HttpResponse::Ok().json(service.status(()).await?)) | ||
Ok(HttpResponse::Ok().json(service.status(&**db).await?)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love concise code, but is this a tad too magic? I can't know at a glance what &**db
is doing. Are we taking a reference of the result of calling first the Deref
impl of web::Data
and then that of Database
? I'm not a rust noob, and I'm not sure. 😄
Is something like service.status(db.get_ref().connection())
too wordy or less idiomatic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could use db.as_ref()
instead. Would do the same. You'd prefer that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we taking a reference of the result of calling first the
Deref
impl ofweb::Data
and then that ofDatabase
?
Btw, close: The first Deref
takes the Arc
from web::Data
, the second takes T
(Database
) from the Arc
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced this with db.as_ref()
.
693f0e7
to
398b96c
Compare
Directly use the `ConnectionTrait` from sea_orm.
398b96c
to
864f467
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
took me a little while to get through, LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly use the
ConnectionTrait
from sea_orm.