Skip to content

Conversation

liubao68
Copy link
Contributor

@liubao68 liubao68 commented Sep 8, 2017

…consumer chain. add thread id in console log.

…consumer chain. add thread id in console log.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 85.54% when pulling cc1dd07 on liubao68:rule_3 into e6e94ae on ServiceComb:master.

@eric-lee-ltk
Copy link
Contributor

eric-lee-ltk commented Sep 11, 2017

Can this warning show up when the application starts and refuses to start instead of runtime warning?


public Transport getTransport() {
if (endpoint == null) {
throw new IllegalStateException("Endpoint is empty. Forget to configure \"loadbalance\" in consumer handler chain?");
Copy link
Member

@WillemJiang WillemJiang Sep 11, 2017

Choose a reason for hiding this comment

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

Can we change the message to "Endpoint is empty. Please configure "loadbalance" in the consumer handler chain."?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess this exception should be detected at startup time and should not be thrown at run time. Validate the microservice.yaml file before the application starts.

Copy link
Contributor

Choose a reason for hiding this comment

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

we don't know which handler is loadBalance
because cutomer can dev their own loadBalance

Copy link
Contributor Author

@liubao68 liubao68 Sep 11, 2017

Choose a reason for hiding this comment

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

Yes, we don't know at which handler will a developer to set the address, and they may not use 'loadbalance' and define a address resolver themself. So this message is a best guess and don't use please or throw an error at startup.

@WillemJiang WillemJiang merged commit c695754 into apache:master Sep 14, 2017
@liubao68 liubao68 deleted the rule_3 branch January 5, 2018 07:36
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.

5 participants