Better connection use in setClassLevelPermissions #4460
Conversation
Method `setClassLevelPermissions` should use `.task` to share the connection for the two consecutive operations. It doesn't need a transaction, because the first operation does not need to roll back when the second one fails.
| const self = this; | ||
| return this._client.task('set-class-level-permissions', function * (t) { | ||
| yield self._ensureSchemaCollectionExists(t); | ||
| const values = [className, 'schema', 'classLevelPermissions', JSON.stringify(CLPs)]; |
There was a problem hiding this comment.
Should we extract the values out of the generator?
There was a problem hiding this comment.
@flovilmart No. This is one of so many now refactored lines where the end result is returned from .none which resolves with null, in order to chain it to the result of the task/transaction, so if the last query fails - we get the correct error. But when we replace it with yield, the errors are thrown, and thus chained to the result automatically, so you do not need to return the value when the callers do not care about the value, only whether it is success or error, i.e. with the last yield you only do return when the resolved value from the task/transaction matters.
There was a problem hiding this comment.
@flovilmart If this is ok, then please review it, as I've got another one ready to go. I'm on a roll this Christmas 😄
Codecov Report
@@ Coverage Diff @@
## master #4460 +/- ##
==========================================
+ Coverage 92.66% 92.68% +0.01%
==========================================
Files 118 118
Lines 8349 8351 +2
==========================================
+ Hits 7737 7740 +3
+ Misses 612 611 -1
Continue to review full report at Codecov.
|
|
All good! |
|
@vitaly-t just flipping through these now, seems I missed the sudden influx of Xmas PRs 😄 , thanks! |
|
@montymxb just my notion of the Christmas spirit 😄 |
Method `setClassLevelPermissions` should use `.task` to share the connection for the two consecutive operations. It doesn't need a transaction, because the first operation does not need to roll back when the second one fails.
Method `setClassLevelPermissions` should use `.task` to share the connection for the two consecutive operations. It doesn't need a transaction, because the first operation does not need to roll back when the second one fails.
Method
setClassLevelPermissionsshould use.taskto share the connection for the two consecutive operations. It doesn't need a transaction, because the first operation does not need to roll back when the second one fails.