Improve logging and error handling, #5
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This overhauls the logging and error responses from the *Conn. Namely, it changes the logging backend to `log/slog` which makes it easier to manage the library's logging centrally (however the old functionally to provide a custom logger is retained). It also captures the reason why a specific connection to a ZK server was closed in the error that is propagated to all callers. This can be checked with `errors.Is`. This also adds an exponential backoff feature to the client, which will make it back off when reconnecting to a ZK server.
bohhyang
approved these changes
Jan 31, 2024
bohhyang
approved these changes
Feb 1, 2024
shivamgupta1
reviewed
Feb 1, 2024
|
Tbqh I don't think the upstream is managed at all, there are no active
maintainers as far as I could tell. I asked LinkedIn to fork this repo so
we would become official maintainers.
…On Wed, Jan 31, 2024 at 19:22 Shivam Gupta ***@***.***> wrote:
***@***.**** commented on this pull request.
Not sure about this one. It's a big change and it's already getting
further and further away from the upstream (9 commits behind). It'll be a
big pain to merge the upstream changes and we might miss out on important
bug fixes and improvements.
—
Reply to this email directly, view it on GitHub
<#5 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABHDA2PFFVBIGSY34AC4OITYRMC6JAVCNFSM6AAAAABCT5EDP2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJVGMYDOOJXGA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Take a look at the repo this repo is itself a fork of:
https://github.com/go-zookeeper/zk , and it itself is behind.
…On Wed, Jan 31, 2024 at 19:23 Paul Chesnais ***@***.***> wrote:
Tbqh I don't think the upstream is managed at all, there are no active
maintainers as far as I could tell. I asked LinkedIn to fork this repo so
we would become official maintainers.
On Wed, Jan 31, 2024 at 19:22 Shivam Gupta ***@***.***>
wrote:
> ***@***.**** commented on this pull request.
>
> Not sure about this one. It's a big change and it's already getting
> further and further away from the upstream (9 commits behind). It'll be a
> big pain to merge the upstream changes and we might miss out on important
> bug fixes and improvements.
>
> —
> Reply to this email directly, view it on GitHub
> <#5 (review)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ABHDA2PFFVBIGSY34AC4OITYRMC6JAVCNFSM6AAAAABCT5EDP2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQNJVGMYDOOJXGA>
> .
> You are receiving this because you authored the thread.Message ID:
> ***@***.***>
>
|
shivamgupta1
reviewed
Feb 1, 2024
|
@PapaCharlie https://github.com/samuel/go-zookeeper is the one that seems unmaintained (last commits were 4 years ago). https://github.com/go-zookeeper/zk seems to have had recent commits (3 months ago). Do you mind double checking this, and confirming that indeed there are no owners? Also, how's the process for becoming official maintainers going to be like? Nevertheless, it might need to be discussed with the management (if not already done) in case we are trying to take up ownership of a large project that most people in the team don't know much about. |
|
These are the 8 commits that we're behind on: |
PapaCharlie
changed the title
PapaCharlie
changed the title
shivamgupta1
reviewed
Feb 2, 2024
bohhyang
reviewed
Feb 2, 2024
PapaCharlie
changed the title
Merged
bohhyang
approved these changes
Feb 2, 2024
PapaCharlie
added a commit
that referenced
this pull request
Feb 5, 2024
* Improve logging and error handling This overhauls the logging and error responses from the *Conn. Namely, it changes the logging backend to `log/slog` which makes it easier to manage the library's logging centrally (however the old functionally to provide a custom logger is retained). It also captures the reason why a specific connection to a ZK server was closed in the error that is propagated to all callers. This can be checked with `errors.Is`. This also adds an exponential backoff feature to the client, which will make it back off when reconnecting to a ZK server. * Remove logger option, it messes with the logging * Change default behavior of DNSHostProvider * Split implementation of HostProvider, fix up tests
PapaCharlie
added a commit
that referenced
this pull request
Feb 5, 2024
* Improve logging and error handling This overhauls the logging and error responses from the *Conn. Namely, it changes the logging backend to `log/slog` which makes it easier to manage the library's logging centrally (however the old functionally to provide a custom logger is retained). It also captures the reason why a specific connection to a ZK server was closed in the error that is propagated to all callers. This can be checked with `errors.Is`. This also adds an exponential backoff feature to the client, which will make it back off when reconnecting to a ZK server. * Remove logger option, it messes with the logging * Change default behavior of DNSHostProvider * Split implementation of HostProvider, fix up tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overhauls the logging and error responses from the
*Conn. Namely, it changes the logging backend tolog/slogwhich makes it easier to manage the library's logging centrally. It also captures the reason why a specific connection to a ZK server was closed in the error that is propagated to all callers. This can be checked witherrors.Is.