snippet-service: rename resourceUpdater config to resourceAggregate #1348
Conversation
WalkthroughThe changes to the Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
charts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamlsnippet-service/config.yamlis excluded by!**/*.yaml
Files selected for processing (1)
- snippet-service/service/config.go (1 hunks)
jkralik
force-pushed
the
jkralik/fix/snippet-service-clients-resource-aggregate
branch
from
408c123 to
8b2ac79
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (5)
snippet-service/service/config_test.go (1)
179-179: Inconsistent Renaming:ResourceUpdatertoResourceAggregateThe renaming from
ResourceUpdatertoResourceAggregateis not consistently applied across the codebase. The following instances still useResourceUpdater:
snippet-service/service/config_test.gosnippet-service/service/service.gosnippet-service/test/service.gosnippet-service/service/service_test.gosnippet-service/service/config.gosnippet-service/updater/resourceUpdater.gosnippet-service/service/grpc/server.gosnippet-service/updater/config.gosnippet-service/updater/config_test.gosnippet-service/updater/expiredUpdates.goPlease update all occurrences of
ResourceUpdatertoResourceAggregateto maintain consistency.Analysis chain
LGTM! Ensure consistency across the codebase.
The renaming from
ResourceUpdatertoResourceAggregateis correctly reflected here.However, ensure that this renaming is consistently applied across the entire codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of renaming across the codebase. # Test: Search for any remaining instances of `ResourceUpdater`. rg --type go 'ResourceUpdater'Length of output: 6041
snippet-service/service/service_test.go (4)
111-114: Ensure consistency of CQLDB configuration settings across the codebase.The following instances of the old CQLDB configuration settings were found and need to be updated to reflect the new structure:
snippet-service/test/service.goidentity-store/test/config.goidentity-store/service/service_test.gocertificate-authority/test/service.goPlease update these instances to ensure consistency across the codebase.
Analysis chain
LGTM! Ensure consistency across the codebase.
The CQLDB configuration settings are correctly updated to reflect the new structure.
However, ensure that these changes are consistently applied across the entire codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of CQLDB configuration settings across the codebase. # Test: Search for any remaining instances of the old CQLDB configuration settings. rg --type go 'Storage.*CqlDB'Length of output: 716
195-196: Inconsistent Configuration Settings DetectedThe configuration settings
CleanUpExpiredUpdatesandExtendCronParserBySecondsare found in multiple files. Ensure these settings are consistently updated across the following locations:
snippet-service/updater/resourceUpdater_test.gosnippet-service/service/service.gosnippet-service/service/http/invokeConfiguration_test.gocertificate-authority/service/cleanDatabase_test.gocertificate-authority/service/config_test.goPlease review and update these files to maintain consistency.
Analysis chain
LGTM! Ensure consistency across the codebase.
The configuration settings are correctly updated to reflect the new structure.
However, ensure that these changes are consistently applied across the entire codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of configuration settings across the codebase. # Test: Search for any remaining instances of the old configuration settings. rg --type go 'Storage.*CleanUpExpiredUpdates' rg --type go 'Storage.*ExtendCronParserBySeconds'Length of output: 1509
80-82: Inconsistent Configuration Settings DetectedThe new configuration structure has not been consistently applied across the codebase. The following instances still use the old configuration settings and need to be updated:
snippet-service/test/service.go: Usescfg.Clients.Storage.Use == database.CqlDBidentity-store/service/service_test.go: Usescfg.Clients.Storage.Use = config.ACTIVE_DATABASE()identity-store/test/config.go: Usescfg.Clients.Storage.Use = config.ACTIVE_DATABASE()certificate-authority/test/service.go: Usescfg.Clients.Storage.Embedded.Use == database.CqlDBPlease update these instances to reflect the new configuration structure.
Analysis chain
LGTM! Ensure consistency across the codebase.
The configuration settings are correctly updated to reflect the new structure.
However, ensure that these changes are consistently applied across the entire codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of configuration settings across the codebase. # Test: Search for any remaining instances of the old configuration settings. rg --type go 'Storage.*Use'Length of output: 377
Script:
#!/bin/bash # Get context around the identified lines in the mentioned files to verify the configuration settings. # Context from snippet-service/test/service.go rg --type go -A 5 -B 5 'cfg.Clients.Storage.Use == database.CqlDB' snippet-service/test/service.go # Context from identity-store/service/service_test.go rg --type go -A 5 -B 5 'cfg.Clients.Storage.Use = config.ACTIVE_DATABASE()' identity-store/service/service_test.go # Context from identity-store/test/config.go rg --type go -A 5 -B 5 'cfg.Clients.Storage.Use = config.ACTIVE_DATABASE()' identity-store/test/config.go # Context from certificate-authority/test/service.go rg --type go -A 5 -B 5 'cfg.Clients.Storage.Embedded.Use == database.CqlDB' certificate-authority/test/service.goLength of output: 1839
93-98: Ensure Consistency Across MongoDB Configuration SettingsThe MongoDB configuration settings are not consistently updated across the codebase. The following files still use the old configuration structure:
tools/mongodb/standby-tool/main.gocertificate-authority/test/service.gosnippet-service/test/service.goidentity-store/test/config.goidentity-store/service/service_test.gocloud2cloud-connector/test/test.gocloud2cloud-connector/test/cloudOrigin.gocloud2cloud-gateway/service/service.gocloud2cloud-gateway/store/mongodb/subscription_test.gocloud2cloud-connector/service/service.goPlease update these instances to use the new configuration structure:
database.Config[*storeMongo.Config, *storeCqlDB.Config].
tools/mongodb/standby-tool/main.go: Lines involvingc.Clients.Storage.MongoDB.*certificate-authority/test/service.go: Lines involvingcfg.Clients.Storage.Embedded.MongoDBsnippet-service/test/service.go: Lines involvingcfg.Clients.Storage.MongoDBidentity-store/test/config.go: Lines involvingcfg.Clients.Storage.MongoDBidentity-store/service/service_test.go: Lines involvingcfg.Clients.Storage.MongoDBcloud2cloud-connector/test/test.go: Lines involvingcfg.Clients.Storage.MongoDBcloud2cloud-connector/test/cloudOrigin.go: Lines involvingidCfg.Clients.Storage.MongoDBandc2cConnectorCfg.Clients.Storage.MongoDBcloud2cloud-gateway/service/service.go: Lines involvingcfg.Clients.Storage.MongoDBcloud2cloud-gateway/store/mongodb/subscription_test.go: Lines involvingcfg.Clients.Storage.MongoDBcloud2cloud-connector/service/service.go: Lines involvingconfig.Clients.Storage.MongoDBPlease ensure these changes are made to maintain consistency across the codebase.
Analysis chain
LGTM! Ensure consistency across the codebase.
The MongoDB configuration settings are correctly updated to reflect the new structure.
However, ensure that these changes are consistently applied across the entire codebase.
Let's gather more information to verify if the new MongoDB configuration structure is consistently applied across the codebase.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of MongoDB configuration settings across the codebase. # Test: Search for any remaining instances of the old MongoDB configuration settings. rg --type go 'Storage.*MongoDB'Length of output: 3597
Script:
#!/bin/bash # Description: Verify the context of MongoDB configuration settings to ensure they follow the new structure. # Check context around the instances found in the previous search. rg --type go -A 5 -B 5 'Storage.*MongoDB' tools/mongodb/standby-tool/main.go rg --type go -A 5 -B 5 'Storage.*MongoDB' certificate-authority/test/service.go rg --type go -A 5 -B 5 'Storage.*MongoDB' snippet-service/test/service.go rg --type go -A 5 -B 5 'Storage.*MongoDB' identity-store/test/config.go rg --type go -A 5 -B 5 'Storage.*MongoDB' identity-store/service/service_test.go rg --type go -A 5 -B 5 'Storage.*MongoDB' cloud2cloud-connector/test/test.go rg --type go -A 5 -B 5 'Storage.*MongoDB' cloud2cloud-connector/test/cloudOrigin.go rg --type go -A 5 -B 5 'Storage.*MongoDB' cloud2cloud-gateway/service/service.go rg --type go -A 5 -B 5 'Storage.*MongoDB' cloud2cloud-gateway/store/mongodb/subscription_test.go rg --type go -A 5 -B 5 'Storage.*MongoDB' cloud2cloud-connector/service/service.goLength of output: 9006
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
charts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamlsnippet-service/config.yamlis excluded by!**/*.yaml
Files selected for processing (11)
- snippet-service/service/config.go (2 hunks)
- snippet-service/service/config_test.go (1 hunks)
- snippet-service/service/http/invokeConfiguration_test.go (1 hunks)
- snippet-service/service/service.go (1 hunks)
- snippet-service/service/service_test.go (5 hunks)
- snippet-service/store/config/config.go (1 hunks)
- snippet-service/test/service.go (2 hunks)
- snippet-service/updater/config.go (1 hunks)
- snippet-service/updater/config_test.go (2 hunks)
- snippet-service/updater/resourceUpdater.go (2 hunks)
- snippet-service/updater/resourceUpdater_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- snippet-service/updater/config.go
Files skipped from review as they are similar to previous changes (1)
- snippet-service/service/config.go
Additional comments not posted (12)
snippet-service/updater/config_test.go (1)
19-19: LGTM!The update to use
MakeResourceAggregateConfigreflects the renaming and maintains the validity of the test case.snippet-service/store/config/config.go (3)
4-5: LGTM!The imports for
fmtandtimeare necessary for the new validation logic involving cron job scheduling.
14-18: LGTM!The addition of
CleanUpExpiredUpdatesandExtendCronParserBySecondsfields to theConfigstruct is necessary for configuring cron job settings and extending the cron parser by seconds.
20-43: LGTM!The added validation logic ensures that the cron job settings are correctly configured and handles errors appropriately.
snippet-service/updater/resourceUpdater_test.go (1)
36-37: LGTM!The update to the configuration paths from
ResourceUpdatertoStoragereflects the restructuring of the configuration settings and ensures that the test cases are aligned with the new configuration paths.snippet-service/test/service.go (2)
44-46: LGTM!The renaming of
MakeResourceUpdaterConfigtoMakeResourceAggregateConfigand the update to the function implementation reflect the new naming convention and ensure consistency.
58-81: LGTM!The updates to the
MakeClientsConfigandMakeStoreConfigfunctions ensure that the helper functions are aligned with the new configuration settings and support the updated structure.snippet-service/service/service.go (1)
99-99: LGTM! Verify the function usage.The additional parameters for
NewResourceUpdaterare correctly passed.However, ensure that all calls to
NewResourceUpdaterare updated to match the new signature.Verification successful
Ensure all calls to
NewResourceUpdaterare updated.The search results show that the function definition and one call to
NewResourceUpdatermatch the new signature. To confirm, please verify that there are no other calls toNewResourceUpdaterusing the old signature.
Verification successful.
All calls to
NewResourceUpdatermatch the new signature.
snippet-service/service/service.gosnippet-service/updater/resourceUpdater.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `NewResourceUpdater` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go 'NewResourceUpdater'Length of output: 612
Script:
#!/bin/bash # Description: Verify all calls to `NewResourceUpdater` match the new signature. # Test: Search for all function calls to `NewResourceUpdater` and check for the correct signature. rg --type go 'NewResourceUpdater' -A 3Length of output: 1145
snippet-service/service/http/invokeConfiguration_test.go (1)
126-127: LGTM! Ensure consistency across the codebase.The configuration paths are correctly updated to reflect the new structure.
However, ensure that these changes are consistently applied across the entire codebase.
Verification successful
Let's verify the context in
snippet-service/service/service.goto ensure that the configuration paths are correctly updated.
Verification Successful: Configuration Paths Consistently Updated
The configuration paths
config.Clients.Storage.CleanUpExpiredUpdatesandconfig.Clients.Storage.ExtendCronParserBySecondshave been correctly updated and are consistent across the codebase.
snippet-service/service/service.go: Lines where the configuration paths are used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of configuration path updates across the codebase. # Test: Search for any remaining instances of `CleanUpExpiredUpdates` and `ExtendCronParserBySeconds` under `ResourceUpdater`. rg --type go 'ResourceUpdater.*CleanUpExpiredUpdates' rg --type go 'ResourceUpdater.*ExtendCronParserBySeconds'Length of output: 641
Script:
#!/bin/bash # Description: Verify the context of configuration path updates in service.go. # Test: Search for the relevant lines in service.go to confirm the updates. rg --type go -A 5 'CleanUpExpiredUpdates' snippet-service/service/service.go rg --type go -A 5 'ExtendCronParserBySeconds' snippet-service/service/service.goLength of output: 899
snippet-service/service/service_test.go (1)
133-133: LGTM! Ensure consistency across the codebase.The resource aggregate client configuration settings are correctly updated to reflect the new structure.
However, ensure that these changes are consistently applied across the entire codebase.
Verification successful
Verified Consistency of ResourceUpdaterConfig Changes
The
ResourceUpdaterConfigandResourceUpdaterstructures are consistently applied across the codebase.
- The
ResourceUpdaterConfigstruct is correctly defined and used in various files.- The
ResourceUpdaterstruct and its methods are consistent with the new structure.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistency of resource aggregate client configuration settings across the codebase. # Test: Search for any remaining instances of the old resource aggregate client configuration settings. rg --type go 'ResourceUpdater'Length of output: 6041
Script:
#!/bin/bash # Description: Extract context around occurrences of ResourceUpdaterConfig and ResourceUpdater to verify consistency. # Extract context around ResourceUpdaterConfig rg --type go 'ResourceUpdaterConfig' -A 5 -B 5 # Extract context around ResourceUpdater rg --type go 'ResourceUpdater' -A 5 -B 5Length of output: 36026
snippet-service/updater/resourceUpdater.go (2)
Line range hint
40-55:
LGTM!The code changes are approved.
40-40: Verify the function usage in the codebase.The function signature has been modified. Ensure that all calls to
NewResourceUpdaterare updated to match the new signature.Verification successful
Verification successful.
The function
NewResourceUpdaterhas been updated correctly, and all calls to this function match the new signature.
snippet-service/updater/resourceUpdater.gosnippet-service/service/service.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `NewResourceUpdater` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'NewResourceUpdater'Length of output: 1335
jkralik
force-pushed
the
jkralik/fix/snippet-service-clients-resource-aggregate
branch
from
55d43f0 to
3b3c3b4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
charts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamlsnippet-service/config.yamlis excluded by!**/*.yaml
Files selected for processing (7)
- snippet-service/service/http/invokeConfiguration_test.go (1 hunks)
- snippet-service/service/service.go (1 hunks)
- snippet-service/service/service_test.go (5 hunks)
- snippet-service/store/config/config.go (1 hunks)
- snippet-service/test/service.go (2 hunks)
- snippet-service/updater/config.go (1 hunks)
- snippet-service/updater/resourceUpdater_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- snippet-service/service/http/invokeConfiguration_test.go
Files skipped from review as they are similar to previous changes (6)
- snippet-service/service/service.go
- snippet-service/service/service_test.go
- snippet-service/store/config/config.go
- snippet-service/test/service.go
- snippet-service/updater/config.go
- snippet-service/updater/resourceUpdater_test.go
jkralik
force-pushed
the
jkralik/fix/snippet-service-clients-resource-aggregate
branch
from
3b3c3b4 to
b935749
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
snippet-service/service/expiredUpdatesChecker.go (1)
1-1: Package name change not fully propagated.The package name has not been fully updated from
updatertoservice. The following files still reference theupdaterpackage and need to be updated:
snippet-service/updater/resourceUpdater_test.gosnippet-service/updater/resourceUpdater.gosnippet-service/updater/updateExecution.gosnippet-service/updater/config_test.gosnippet-service/updater/config.goPlease ensure all references to the
updaterpackage are updated toserviceto maintain consistency.Analysis chain
Package name change.
The package name has been changed from
updatertoservice. Ensure all references to this package are updated accordingly.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all references to the package `updater` are updated to `service`. # Test: Search for the package usage. Expect: Only occurrences of `service`. rg --type go -A 5 $'package updater'Length of output: 1698
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
charts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamlsnippet-service/config.yamlis excluded by!**/*.yaml
Files selected for processing (10)
- snippet-service/service/expiredUpdatesChecker.go (2 hunks)
- snippet-service/service/http/invokeConfiguration_test.go (1 hunks)
- snippet-service/service/service.go (6 hunks)
- snippet-service/service/service_test.go (5 hunks)
- snippet-service/store/config/config.go (1 hunks)
- snippet-service/test/service.go (2 hunks)
- snippet-service/updater/config.go (1 hunks)
- snippet-service/updater/config_test.go (2 hunks)
- snippet-service/updater/resourceUpdater.go (4 hunks)
- snippet-service/updater/resourceUpdater_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (9)
- snippet-service/service/http/invokeConfiguration_test.go
- snippet-service/service/service.go
- snippet-service/service/service_test.go
- snippet-service/store/config/config.go
- snippet-service/test/service.go
- snippet-service/updater/config.go
- snippet-service/updater/config_test.go
- snippet-service/updater/resourceUpdater.go
- snippet-service/updater/resourceUpdater_test.go
Additional comments not posted (4)
snippet-service/service/expiredUpdatesChecker.go (4)
10-10: Error handling forgocron.NewScheduler.The error handling for
gocron.NewScheduleris correct and ensures that any errors in creating the scheduler are properly reported.
15-15: Cron job creation and error handling.The cron job creation and error handling are correct. The use of
gocron.NewTask(onCheckTimeout)ensures that the provided function is correctly scheduled.
15-15: Starting the scheduler.The scheduler is correctly started with
s.Start(). Ensure that this does not lead to any unintended side effects.
10-10: Function signature update.The function signature has been updated to replace the
updaterparameter withonCheckTimeout. Ensure all calls to this function are updated accordingly.Verification successful
Function signature update verified.
All calls to
NewExpiredUpdatesCheckerhave been updated to match the new signature.
snippet-service/service/service.gosnippet-service/service/expiredUpdatesChecker.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all calls to `NewExpiredUpdatesChecker` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type go -A 5 $'NewExpiredUpdatesChecker'Length of output: 1219
Script:
#!/bin/bash # Description: Verify all calls to `NewExpiredUpdatesChecker` match the new signature. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'NewExpiredUpdatesChecker'Length of output: 370
Danielius1922
force-pushed
the
jkralik/fix/snippet-service-clients-resource-aggregate
branch
from
b935749 to
e2a56ad
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
charts/plgd-hub/templates/snippet-service/config.yamlis excluded by!**/*.yamlcharts/plgd-hub/values.yamlis excluded by!**/*.yamlsnippet-service/config.yamlis excluded by!**/*.yaml
Files selected for processing (11)
- snippet-service/service/config.go (3 hunks)
- snippet-service/service/config_test.go (3 hunks)
- snippet-service/service/expiredUpdatesChecker.go (2 hunks)
- snippet-service/service/http/invokeConfiguration_test.go (1 hunks)
- snippet-service/service/service.go (6 hunks)
- snippet-service/service/service_test.go (6 hunks)
- snippet-service/store/config/config.go (1 hunks)
- snippet-service/store/config/config_test.go (1 hunks)
- snippet-service/test/service.go (3 hunks)
- snippet-service/updater/resourceUpdater.go (4 hunks)
- snippet-service/updater/resourceUpdater_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- snippet-service/store/config/config_test.go
Files skipped from review as they are similar to previous changes (9)
- snippet-service/service/config.go
- snippet-service/service/config_test.go
- snippet-service/service/expiredUpdatesChecker.go
- snippet-service/service/http/invokeConfiguration_test.go
- snippet-service/service/service.go
- snippet-service/store/config/config.go
- snippet-service/test/service.go
- snippet-service/updater/resourceUpdater.go
- snippet-service/updater/resourceUpdater_test.go
Additional comments not posted (1)
snippet-service/service/service_test.go (1)
79-81: Review of Updated Configurations in Test CasesThe test cases have been updated to reflect the new configuration structures and naming conventions. However, there are several points to consider:
Invalid Configurations Testing: The tests for invalid configurations (lines 79-81, 92-97, 110-113) are correctly set up to expect errors, which is good practice for negative testing.
Expired Updates Checker Config: The configuration for expired updates checker (lines 119-127) uses a hardcoded invalid value for testing error scenarios. This is a good use of testing edge cases.
Resource Aggregate Config: The renaming of
ResourceUpdaterConfigtoResourceAggregateConfig(line 141) is consistent with the changes described in the PR. Ensure all references in the codebase are updated to reflect this change.Overall, the test configurations are well-handled and align with the new system architecture. However, ensure comprehensive coverage of all possible configurations to avoid potential issues in production.
Also applies to: 92-97, 110-113, 119-127, 141-141
|
Danielius1922
deleted the
jkralik/fix/snippet-service-clients-resource-aggregate
branch
No description provided.