Skip to content

Conversation

@dfawley
Copy link
Member

@dfawley dfawley commented Aug 1, 2023

Part of #6472

This is a minimal change to migrate pick_first to the StateNotifier. Since PF is slated to become a lot more complex, I didn't want to do any significant refactoring here.

RELEASE NOTES: none

@dfawley dfawley added the Type: Internal Cleanup Refactors, etc label Aug 1, 2023
@dfawley dfawley added this to the 1.58 Release milestone Aug 1, 2023
@dfawley dfawley requested a review from easwars August 1, 2023 22:22
// UpdateSubConnState is unused as a StateListener is always registered when
// creating SubConns.
func (b *pickfirstBalancer) UpdateSubConnState(subConn balancer.SubConn, state balancer.SubConnState) {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we throw an error log here, just to ensure that it is caught by our tests if by any chance this ends up happening?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea! Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@easwars easwars assigned dfawley and unassigned easwars Aug 1, 2023
// UpdateSubConnState is unused as a StateListener is always registered when
// creating SubConns.
func (b *pickfirstBalancer) UpdateSubConnState(subConn balancer.SubConn, state balancer.SubConnState) {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@dfawley dfawley merged commit d06ab0d into grpc:master Aug 4, 2023
@dfawley dfawley deleted the StateListener1 branch August 4, 2023 15:14
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants