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

Support apollo config injection in Spring Boot bootstrap phase #937

Merged
merged 3 commits into from Jan 28, 2018

Conversation

nobodyiam
Copy link
Member

@nobodyiam nobodyiam commented Jan 28, 2018

Some cases like @ConditionalOnProperty use the config values earlier than the phase when Apollo configs are injected(postProcessBeanFactory), so a new way to inject the Apollo config in Spring Boot bootstrap phase is provided.

Configuration example:

     # will inject 'application' namespace in bootstrap phase
     apollo.bootstrap.enabled = true

or

     apollo.bootstrap.enabled = true
     # will inject 'application' and 'FX.apollo' namespaces in bootstrap phase
     apollo.bootstrap.namespaces = application,FX.apollo

胡昇 and others added 2 commits January 28, 2018 22:08
2. Reorganize the feature to provide the ability to inject apollo
configs in Spring Boot bootstrap phase
3. Add more integration tests to cover different scenarios
4. Update apollo-demo to show this apollo bootstrap config feature could
be used with @ConditionalOnProperty
@codecov-io
Copy link

codecov-io commented Jan 28, 2018

Codecov Report

Merging #937 into master will decrease coverage by 3.56%.
The diff coverage is 89.18%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #937      +/-   ##
============================================
- Coverage     50.48%   46.91%   -3.57%     
- Complexity      810     1554     +744     
============================================
  Files           175      354     +179     
  Lines          4887     9755    +4868     
  Branches        472      968     +496     
============================================
+ Hits           2467     4577    +2110     
- Misses         2221     4835    +2614     
- Partials        199      343     +144
Impacted Files Coverage Δ Complexity Δ
...apollo/spring/config/PropertySourcesProcessor.java 100% <100%> (ø) 12 <5> (+1) ⬆️
...rk/apollo/spring/boot/ApolloAutoConfiguration.java 100% <100%> (ø) 2 <2> (?)
...pring/boot/ApolloSpringApplicationRunListener.java 85.71% <85.71%> (ø) 9 <9> (?)
...lo/portal/spi/configuration/AuthConfiguration.java 10.1% <0%> (ø) 1% <0%> (?)
...ramework/apollo/openapi/util/ConsumerAuthUtil.java 100% <0%> (ø) 4% <0%> (?)
...apollo/adminservice/aop/NamespaceUnlockAspect.java 59.57% <0%> (ø) 12% <0%> (?)
...k/apollo/portal/component/RestTemplateFactory.java 100% <0%> (ø) 5% <0%> (?)
...lo/configservice/util/InstanceConfigAuditUtil.java 77.19% <0%> (ø) 15% <0%> (?)
...trip/framework/apollo/openapi/entity/Consumer.java 45.83% <0%> (ø) 7% <0%> (?)
...mework/apollo/openapi/service/ConsumerService.java 41.93% <0%> (ø) 11% <0%> (?)
... and 173 more

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 39dc297...cbb0224. Read the comment docs.

logger.debug("Apollo bootstrap config is not enabled for context {}, see property: ${{}}", context, PropertySourcesConstants.APOLLO_BOOTSTRAP_ENABLED);
return;
}
logger.debug("Apollo bootstrap config is enabled for context {}", context, PropertySourcesConstants.APOLLO_BOOTSTRAP_ENABLED);

Choose a reason for hiding this comment

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

这句日志少了一个大括号{}

Copy link
Member Author

Choose a reason for hiding this comment

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

。。。这个日志不需要传PropertySourcesConstants.APOLLO_BOOTSTRAP_ENABLED这个参数。。。可以提个PR改一下。。。或者晚点我改一下也行

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants