Skip to content

Conversation

vicancy
Copy link
Member

@vicancy vicancy commented Jun 17, 2019

No description provided.

}

public Task StartAsync(string target = null)
{
ConnectionStatusChanged?.Invoke(new StatusChange(ServiceConnectionStatus.Inited, Status));
Copy link
Contributor

Choose a reason for hiding this comment

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

Inited [](start = 85, length = 6)

why added an "Inited" status but only used in test? not Connecting?

Copy link
Member Author

Choose a reason for hiding this comment

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

inited status is added for ServiceConnectionContainer to have an initial state Inited instead of Disconnected as the initial state, so that ServiceConnectionContainer can get notified when the inner server connections has status changed to Disconnected. And I am not able to reuse Connecting as for backward compatibility, the status of ServiceConnectionContainer should be considered as online until it starts the inner service connection.

Copy link
Member Author

Choose a reason for hiding this comment

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

So with inited we explicitly defined a stage for the server connection between it is created to it is connected. I think it is clear, as for now, we consider the endpoint as online in this stage, which is when the app server starts. And later, we can easily change the endpoint status for this stage if there is a requirement.

Copy link
Contributor

@JialinXin JialinXin left a comment

Choose a reason for hiding this comment

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

:shipit:

@vicancy vicancy merged commit b35c00f into Azure:dev Jun 19, 2019
@vicancy vicancy deleted the pm branch June 19, 2019 05:22
@vicancy vicancy mentioned this pull request Jul 17, 2019
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.

2 participants