Skip to content
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

add a config adjust the property source overriden behavior #4377

Closed

Conversation

shenhuaxin
Copy link
Contributor

What's the purpose of this PR

add a config adjust the property source overriden behavior

Which issue(s) this PR fixes:

Fixes #4367 (comment)

Brief changelog

add property apollo.bootstrap.overrideSystemProperties change apollo property source overriden behavior

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Read the Contributing Guide before making this pull request.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Write necessary unit tests to verify the code.
  • Run mvn clean test to make sure this pull request doesn't break anything.
  • Update the CHANGES log.

@github-actions
Copy link

github-actions bot commented May 24, 2022

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@shenhuaxin
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Member

@nobodyiam nobodyiam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! This looks good to me.
However, I think this config should also work for non-spring-boot environment, so we need to rename the config name(apollo.bootstrap is for configs only applicable to spring boot).
Also we need to tell the user how to config it in java-sdk-user-guide(zh, en).

@shenhuaxin
Copy link
Contributor Author

  1. rename apollo.bootstrap.overrideSystemProperties to apollo.overrideSystemProperties
  2. add non-spring-boot environment support

@nobodyiam nobodyiam added this to the 2.1.0 milestone May 29, 2022
@nobodyiam
Copy link
Member

I think we could merge this pr after 2.0.1 is released, meanwhile, would you please help to java-sdk-user-guide(zh, en) to tell user how to use this feature?

@shenhuaxin
Copy link
Contributor Author

sure, i will do it

@codecov-commenter
Copy link

Codecov Report

Merging #4377 (60c15ce) into master (1cdbd14) will increase coverage by 0.00%.
The diff coverage is 80.00%.

@@            Coverage Diff            @@
##             master    #4377   +/-   ##
=========================================
  Coverage     53.18%   53.18%           
- Complexity     2674     2676    +2     
=========================================
  Files           489      489           
  Lines         15277    15291   +14     
  Branches       1580     1585    +5     
=========================================
+ Hits           8125     8133    +8     
- Misses         6597     6603    +6     
  Partials        555      555           
Impacted Files Coverage Δ
...ramework/apollo/core/ApolloClientSystemConsts.java 0.00% <ø> (ø)
...apollo/spring/config/PropertySourcesProcessor.java 93.54% <66.66%> (-2.95%) ⬇️
...ring/boot/ApolloApplicationContextInitializer.java 92.98% <75.00%> (-1.36%) ⬇️
...va/com/ctrip/framework/apollo/util/ConfigUtil.java 82.63% <100.00%> (+0.46%) ⬆️
...framework/apollo/openapi/entity/ConsumerAudit.java 42.42% <0.00%> (-6.07%) ⬇️
...mework/apollo/openapi/service/ConsumerService.java 53.38% <0.00%> (-1.70%) ⬇️
...ervice/service/ReleaseMessageServiceWithCache.java 87.05% <0.00%> (+1.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cdbd14...60c15ce. Read the comment docs.

@nobodyiam
Copy link
Member

As 2.1.0 is released, please rebase the code and also update the CHANGES.md.

You may see changes-2.0.1 for reference.

nobodyiam and others added 5 commits June 10, 2022 09:30
@shenhuaxin
Copy link
Contributor Author

As 2.1.0 is released, please rebase the code and also update the CHANGES.md.

You may see changes-2.0.1 for reference.

i did something wrong with rebase , and i don't know how to fix it , may i need to create another pr to fix it ?

@nobodyiam
Copy link
Member

@shenhuaxin

If rebase doesn't work for you, you may use squash, see the CONTRIBUTING guide.
A new pr is also OK.

image

@shenhuaxin
Copy link
Contributor Author

this is a new pr. #4409
image

i cann't combine the two comimt into one, if i rebase and squash ff1a2e9, push is reject and need merge. can you give me some advice?

@nobodyiam
Copy link
Member

this is a new pr. #4409 image

i cann't combine the two comimt into one, if i rebase and squash ff1a2e9, push is reject and need merge. can you give me some advice?

You may use force push to your shenhuaxin:apollo-pr-override-systemproperties branch. But it's ok as we could squash and merge the pr.

@shenhuaxin shenhuaxin closed this Jun 11, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spring boot的配置优先级顺序
5 participants