-
Notifications
You must be signed in to change notification settings - Fork 827
Jav 123 rest tracing #60
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
Conversation
7558aea
to
c18649e
Compare
b6194d0
to
9ddf15e
Compare
9ddf15e
to
b132725
Compare
b132725
to
336f5fb
Compare
29b2156
to
7a56071
Compare
…notation to intercept existing rest templates
3467fa0
to
7f82fb9
Compare
} | ||
return properties; | ||
} | ||
|
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.
ConfigUtil.installDynamicConfig() is duplicate with bean inialization.
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.
sorry, could you elaborate a bit? I didn't get what you mean.
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.
这个只是逻辑上有点绕,感觉不是很顺,功能上没问题。ConfigUtil.installDynamicConfig和DynamicPropertiesImpl(AbstractConfiguration... configurations) 做的是同一个事情。DynamicPropertiesImpl只不过现在初始化的时候,没有调用ConfiguratioinManager.install。 从整个系统来看,初始化配置的过程还是由ConfigUtil.installDynamicConfig完成的。
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.
the idea of DynamicProperties here is to replace direct use of archaius classes in our code, so that the testing and extending our framework to support other dynamic configurations, such as disconf, can be done without modification to our framework code.
DynamicPropertiesImpl() { | ||
} | ||
|
||
DynamicPropertiesImpl(AbstractConfiguration... configurations) { |
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.
unused consturctor
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.
this constructor is called in testing
import com.netflix.config.ConcurrentMapConfiguration; | ||
import com.netflix.config.ConfigurationManager; | ||
import com.netflix.config.DynamicPropertyFactory; | ||
import com.seanyinx.github.unit.scaffolding.Poller; |
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.
this code(com.seanyinx.*) , is it persional code ? is it ok?
@liubao68 @wujimin