Skip to content

Conversation

@yhs0092
Copy link
Member

@yhs0092 yhs0092 commented Dec 3, 2017

I've thought about several solutions on this issue.

  • move sending AFTER_REGISTRY logic into ServiceCenterTask. But this means ServiceCenterTask need to get the instance of CseApplicationListener, and triggerEvent() of CseApplicationListener has to be changed to public.
  • move sending AFTER_REGISTRY logic into a call-back function and pass it to ServiceCenterTask. When instance registry is completed, this function should be called. But this will change several Class from RegistryUtils to ServiceCenterTask.

I think both of them are not good. So I choose to write an anonymous subscriber and let it to listen to MicroserviceInstanceRegisterTask. Maybe this is the solution with the minimum change of code.

…ry event and trigger AFTER_REGISTRY event.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 86.906% when pulling bad5fe8 on yhs0092:JAV-546_set_After_Registry_Trigger_Time into 7e4bbc4 on ServiceComb:master.

schemaListenerManager.notifySchemaListener();

triggerEvent(EventType.BEFORE_REGISTRY);
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

it's better to write new comments in english.
and maybe this logic can be a new method, only invoke in this place.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've extracted this part into a new private method, and translated the comments into English. Please review it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 86.907% when pulling 33e793d on yhs0092:JAV-546_set_After_Registry_Trigger_Time into 7e4bbc4 on ServiceComb:master.

@WillemJiang WillemJiang merged commit 8fc1d15 into apache:master Dec 5, 2017
@yhs0092 yhs0092 deleted the JAV-546_set_After_Registry_Trigger_Time branch July 23, 2018 12:17
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