-
Notifications
You must be signed in to change notification settings - Fork 36
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
Update common_ci.properties #112
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe pull request involves updating the base URL references in 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/environment/common_ci.properties (2)
Line range hint
1-91
: Consider security improvements for sensitive configurationsThe file contains several sensitive configurations:
- Database credentials (though parameterized)
- API keys (Fetosense)
- Authentication details (authKey, authSecret)
Consider:
- Moving sensitive configurations to a secure vault service
- Using encryption for sensitive values
- Implementing runtime secret injection
Line range hint
1-91
: Document the environment variable changesTo ensure smooth deployment across environments:
- Document the new URL pattern changes (
*_API
vs*_API_BASE_URL
)- Update deployment guides and environment setup documentation
- Ensure CI/CD pipelines are updated with the new variable names
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/environment/common_ci.properties
(2 hunks)
🔇 Additional comments (6)
src/main/environment/common_ci.properties (6)
9-9
: LGTM: Carestream URL update follows the new pattern
The URL pattern change from COMMON_API_BASE_URL
to COMMON_API
is consistent with the PR objective.
26-26
: LGTM: Beneficiary and COVID vaccine endpoints updated correctly
The URL updates for beneficiary management and COVID vaccine endpoints follow the new pattern consistently.
Also applies to: 29-29, 32-32
41-41
: LGTM: TC specialist scheduling endpoints updated
The scheduler API endpoint updates are consistent with the new pattern.
Also applies to: 44-44
47-47
: LGTM: SMS and document management URLs updated
The SMS and OpenKM document management endpoint updates follow the new pattern correctly.
Also applies to: 50-50
13-13
: Verify the identity service URL pattern consistency
While the common API URLs have been updated, there's an inconsistency with HWC_IDENTITY_API_BASE
used in other identity-related properties (lines 15, 17). Consider standardizing all identity service URLs.
Also applies to: 19-19, 21-21
35-35
: Verify data sync endpoint configuration
The change from MMU_API_BASE_URL
to MMU_API
looks correct, but please ensure the MMU service is configured to recognize this new environment variable pattern.
Also applies to: 38-38
✅ Verification successful
Based on the analysis of the property files, I can now generate the final response:
Environment variable pattern change is correctly configured
The change from MMU_API_BASE_URL
to MMU_API
is properly configured in the CI environment properties file (common_ci.properties) and is used consistently for both data sync endpoints:
- [email protected]_API@/dataSync/van-to-server
- [email protected]_API@/dataSync/server-to-van
The environment variable pattern follows the same convention as other API endpoints in the file (like COMMON_API, SCHEDULER_API, etc.) and is properly templated for CI environment builds.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any MMU API related configurations
rg "MMU_API|mmu" --type properties
Length of output: 71
Script:
#!/bin/bash
# Search for MMU API related configurations in all files
rg "MMU_API|mmu" -i
# Also specifically look for environment variable patterns
rg "@env\." -A 2 -B 2
Length of output: 72830
Script:
#!/bin/bash
# Search for all property files and their content
fd -e properties -x cat {}
# Also look for environment variable configurations in any config files
fd -e yml -e yaml -e xml -e conf -x cat {}
Length of output: 32877
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note any relevant details for your test configuration.
Checklist:
Summary by CodeRabbit