feat: Enhance multi-cloud support in mesh configuration#1298
Conversation
|
Warning Review limit reached
More reviews will be available in 49 minutes and 55 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds two optional multi-cloud flags to ChangesMulti-cloud configuration and mesh refactoring
Estimated code review effort: Possibly related PRs:
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/mobility-core/src/Kernel/Beam/Functions.hs (1)
170-170: 💤 Low valueMinor: Error message references wrong function name.
The error message says
"Schema not found in setMeshConfig"but this is insetMeshConfigWithTables. Consider updating for accurate debugging:- schema <- maybe (L.throwException $ InternalError "Schema not found in setMeshConfig") pure mSchema + schema <- maybe (L.throwException $ InternalError "Schema not found in setMeshConfigWithTables") pure mSchema🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/Beam/Functions.hs` at line 170, The error message incorrectly names the function; update the InternalError message used in the schema failure branch so it references setMeshConfigWithTables instead of setMeshConfig—specifically modify the string passed to L.throwException/InternalError in the expression that binds schema (the line with schema <- maybe (L.throwException $ InternalError "Schema not found in setMeshConfig") pure mSchema) to read "Schema not found in setMeshConfigWithTables".
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lib/mobility-core/src/Kernel/Beam/Functions.hs`:
- Line 170: The error message incorrectly names the function; update the
InternalError message used in the schema failure branch so it references
setMeshConfigWithTables instead of setMeshConfig—specifically modify the string
passed to L.throwException/InternalError in the expression that binds schema
(the line with schema <- maybe (L.throwException $ InternalError "Schema not
found in setMeshConfig") pure mSchema) to read "Schema not found in
setMeshConfigWithTables".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 46cdd586-2db7-45d4-a95b-f6a87ed3a827
📒 Files selected for processing (2)
lib/mobility-core/src/Kernel/Beam/Functions.hslib/mobility-core/src/Kernel/Types/Common.hs
vijaygupta18
force-pushed
the
backend/enabling-find-all-to-multicloud
branch
from
137b143 to
c4ed194
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/mobility-core/src/Kernel/Types/Common.hs (1)
88-103:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing field initializations will cause compilation failure.
The
defaultTableDatarecord is missing initializations fordrainerTtlConfigsandenableFindAllForMultiCloud. With-Wall -Werrorthis will not compile.🐛 Proposed fix
enableSecondaryCloudRead = Nothing, tablesForSecondaryCloudRead = Nothing, - enableAllTablesForSecondaryCloudRead = Nothing + enableAllTablesForSecondaryCloudRead = Nothing, + drainerTtlConfigs = Nothing, + enableFindAllForMultiCloud = Nothing }As per coding guidelines: "All code must compile without warnings under GHC flags
-Wall -Werror".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/mobility-core/src/Kernel/Types/Common.hs` around lines 88 - 103, The record defaultTableData (of type Tables) is missing initializers for drainerTtlConfigs and enableFindAllForMultiCloud which breaks compilation; update the defaultTableData definition to include those fields (e.g. add drainerTtlConfigs = HM.empty and enableFindAllForMultiCloud = Nothing or the correct default for its type) alongside the other record fields so the Tables record is fully initialized.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@lib/mobility-core/src/Kernel/Types/Common.hs`:
- Around line 88-103: The record defaultTableData (of type Tables) is missing
initializers for drainerTtlConfigs and enableFindAllForMultiCloud which breaks
compilation; update the defaultTableData definition to include those fields
(e.g. add drainerTtlConfigs = HM.empty and enableFindAllForMultiCloud = Nothing
or the correct default for its type) alongside the other record fields so the
Tables record is fully initialized.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 117646a6-d6f9-4fa3-a453-73647be5e13a
📒 Files selected for processing (2)
lib/mobility-core/src/Kernel/Beam/Functions.hslib/mobility-core/src/Kernel/Types/Common.hs
- Introduced `enableFindAllForMultiCloud` option in `Tables` type to control findAll query behavior across multiple clouds. - Refactored `setMeshConfig` and `setMeshConfigForFindAll` to utilize the new `Tables` configuration for improved handling of Redis settings. - Updated comments for clarity on the impact of multi-cloud settings on findAll queries.
vijaygupta18
force-pushed
the
backend/enabling-find-all-to-multicloud
branch
from
c4ed194 to
91e8347
Compare
2 participants
enableFindAllForMultiCloudoption inTablestype to control findAll query behavior across multiple clouds.setMeshConfigandsetMeshConfigForFindAllto utilize the newTablesconfiguration for improved handling of Redis settings.Type of Change
Description
Additional Changes
Motivation and Context
How did you test it?
Checklist
./dev/format-all-files.shSummary by CodeRabbit
New Features
Improvements