-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
enhance to load mocked properties from apollo.cache-dir #58
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
WalkthroughThe recent updates enhance the Apollo Testing Server by improving how configuration data is handled. This includes better resource loading, file operations, and cache management. Additionally, new test setups verify the functionality with custom cache directories, ensuring configurations are loaded correctly from specified paths. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (2)
Files skipped from review as they are similar to previous changes (1)
Additional Context UsedLanguageTool (4)
Additional comments not posted (3)
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 (
|
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: 3
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Show resolved
Hide resolved
I have read the CLA Document and I hereby sign the CLA |
recheck |
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: 3
...ava/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiWhileCacheDirSpecifiedTest.java
Outdated
Show resolved
Hide resolved
...ava/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiWhileCacheDirSpecifiedTest.java
Outdated
Show resolved
Hide resolved
...-mockserver/src/test/java/com/ctrip/framework/apollo/mockserver/ApolloMockServerApiTest.java
Outdated
Show resolved
Hide resolved
有几点疑问可以讨论下:
|
回复如下,个人觉得apollo.cache-dir可能是比较好的使用方式,当然不大了解别的使用场景,可以讨论学习下。
场景:拿apollo.cache-dir下的配置作为baseline,通过apollo. addOrModifyProperty()做极少量的变更来进行分支覆盖。主要是从管理维护配置的角度:
当apollo.cache-dir没有配置时,使用mockdata-*.properties,而getDefaultLocalCacheDir始终返回非空,所以用了getCustomizedCacheRoot,使用getCustomizedCacheRoot时,手工拼接文件夹路径,代码确实有点奇怪。
综上1和2,调整一下优先级,优先使用mockdata-*.properties,不存在时再读取apollo.cache-dir的配置。 |
…ultLocalCacheDir instead
…ultLocalCacheDir instead
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
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.
As this is an enhancement, please also update the CHANGES.md.
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
apollo-mockserver/src/main/java/com/ctrip/framework/apollo/mockserver/ApolloTestingServer.java
Outdated
Show resolved
Hide resolved
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: 2
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.
LGTM
What's the purpose of this PR
当没有配置apollo.cache-dir / apollo.cacheDir时,mock配置properties文件读取classpath下的mockdata-namespace.properties。
当配置了apollo.cache-dir / apollo.cacheDir时,mock配置properties文件从指定位置读取。
Ref #55
I have read the CLA Document and I hereby sign the CLA.
Summary by CodeRabbit
New Features
Tests
Documentation