-
Notifications
You must be signed in to change notification settings - Fork 145
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
[MINOR] Avoid returning null in defaultUserApps
when quota file does't config user
#786
Conversation
defaultUserApps
defaultUserApps
defaultUserApps
defaultUserApps
when quota file does't config user
Codecov Report
@@ Coverage Diff @@
## master #786 +/- ##
============================================
+ Coverage 57.86% 59.31% +1.45%
Complexity 2028 2028
============================================
Files 298 284 -14
Lines 14520 12554 -1966
Branches 1185 1185
============================================
- Hits 8402 7447 -955
+ Misses 5643 4690 -953
+ Partials 475 417 -58
... and 17 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
LGTM. I hadn't noticed this difference between |
Thanks @smallzhongfeng Merged. |
Thanks for your review! @zuston |
…pache#786) ### What changes were proposed in this pull request? `map.getOrDefault()` => `map.computeIfAbsent()` ### Why are the changes needed? If do not define the user and app num in the quota configuration file, it will appear that the `defaultUserApps` does not have the corresponding app number for this user, because we did not include the `quotaAppNum` in the `defaultUserApps`. **before** : The log from coordinator is `[ERROR] 2023-03-28 19:24:11,18 Grpc-260 AccessAppQuotaChecker check - Denied by AccessAppQuotaChecker => User: xxxx, current app num is: 3, default app num is: null.` **after** : The log from coordinator is `[ERROR] 2023-03-31 14:32:21,228 Grpc-240 AccessAppQuotaChecker check - Denied by AccessAppQuotaChecker => User: xxxx, current app num is: 3, default app num is: 3.` ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Existing UTs.
@smallzhongfeng There is a fix. It should be merged to branch 0.7. But there is conflicts with branch 0.7. Could you raise a pr to branch 0.7? |
What changes were proposed in this pull request?
map.getOrDefault()
=>map.computeIfAbsent()
Why are the changes needed?
If do not define the user and app num in the quota configuration file, it will appear that the
defaultUserApps
does not have the corresponding app number for this user, because we did not include thequotaAppNum
in thedefaultUserApps
.before :
The log from coordinator is
[ERROR] 2023-03-28 19:24:11,18 Grpc-260 AccessAppQuotaChecker check - Denied by AccessAppQuotaChecker => User: xxxx, current app num is: 3, default app num is: null.
after :
The log from coordinator is
[ERROR] 2023-03-31 14:32:21,228 Grpc-240 AccessAppQuotaChecker check - Denied by AccessAppQuotaChecker => User: xxxx, current app num is: 3, default app num is: 3.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Origin uts.