chore(app): Temp. disable authorized parties cache #1512
Conversation
📝 WalkthroughWalkthroughThe pull request primarily modifies the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs (1)
85-86: Add more context about the bug impact.While the comment references the bug, it would be helpful to add more context about why caching needed to be disabled and what issues it was causing. This helps future maintainers understand the rationale behind this temporary change.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs(1 hunks)
🔇 Additional comments (1)
src/Digdir.Domain.Dialogporten.Infrastructure/Altinn/Authorization/AltinnAuthorizationClient.cs (1)
84-89: Add TODO to track cache re-enablement.
The caching is temporarily disabled, but there's no clear indicator for when it should be re-enabled. Consider adding a TODO comment to help track this:
// Disabled until this bug is fixed: https://github.com/digdir/dialogporten/issues/1226
+// TODO: Re-enable caching once #1226 is fixed. This temporary disable may impact performance as each call will hit the external service.
// var authorizedParties = await _partiesCache.GetOrSetAsync(authorizedPartiesRequest.GenerateCacheKey(), async token
// => await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, token), token: cancellationToken);
var authorizedParties = await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, cancellationToken);Let's verify the impact of this change by checking for other cache usages and dependencies:
✅ Verification successful
Based on the shell script results, I can now generate the final response as we have sufficient context about caching usage in the codebase.
Add TODO comment to track cache re-enablement and performance impact.
The caching is temporarily disabled due to issue #1226, and the codebase shows extensive use of caching for performance optimization. The suggested comment addition is appropriate as:
- The codebase has detailed caching configurations with performance considerations
- The file shows other caches are actively used for authorization and party information
- The infrastructure setup has specific memory and distributed cache configurations
// Disabled until this bug is fixed: https://github.com/digdir/dialogporten/issues/1226
+// TODO: Re-enable caching once #1226 is fixed. This temporary disable may impact performance as each call will hit the external service.
// var authorizedParties = await _partiesCache.GetOrSetAsync(authorizedPartiesRequest.GenerateCacheKey(), async token
// => await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, token), token: cancellationToken);
var authorizedParties = await PerformAuthorizedPartiesRequest(authorizedPartiesRequest, cancellationToken);🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other cache usages and dependencies
# Look for other uses of _partiesCache
rg "_partiesCache" -A 5
# Look for other caching patterns
rg "GetOrSetAsync" -A 5
# Look for performance-related comments or TODOs
rg -i "performance|cache|todo" -g "*.cs"
Length of output: 30861
|
Description
Related Issue(s)
Verification
Documentation
docs-directory, Altinnpedia or a separate linked PR in altinn-studio-docs., if applicable)Summary by CodeRabbit