Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ClientConfig Logger should be used for utp #676

Closed
anacrolix opened this issue Sep 28, 2021 · 4 comments · Fixed by #722
Closed

ClientConfig Logger should be used for utp #676

anacrolix opened this issue Sep 28, 2021 · 4 comments · Fixed by #722

Comments

@anacrolix
Copy link
Owner

Per #332 (comment), the ClientConfig Logger should be passed through to go-libutp (and possibly anacrolix/utp when that's in use?). Currently it's necessary to modify the global Logger instance manually, and that belongs to the package, not to any utp instances owned by a torrent Client.
@anacrolix anacrolix mentioned this issue Sep 28, 2021
Closed
@FIGBERT
Copy link
Contributor

FIGBERT commented Jan 28, 2022

I'm looking to write a PR to implement the ClientConfig passthrough, but not sure exactly where to start.

Would changing utp.Logger before the initial call to utp be sufficient? How can I access ClientConfig.Logger from utp_libutp.go?

@anacrolix
Copy link
Owner Author

I think it shoudl be passed through in construction of the utp.Socket. Probably an option in NewSocket, or a method to set it immediately. It will need to be extracted from the owning Socket for any logging that occurs in utp. I.e. the global utp Logger should be given to a new socket that doesn't override the default, and should not be referenced anywhere else.

@FIGBERT FIGBERT mentioned this issue Jan 30, 2022
Merged
@FIGBERT
Copy link
Contributor

FIGBERT commented Feb 1, 2022

Once anacrolix/go-libutp releases a new version (see: anacrolix/go-libutp#19), I'll start work on a PR here.

@anacrolix
Copy link
Owner Author

I pushed v1.2.0 for this purpose. Thanks @FIGBERT .

@FIGBERT FIGBERT mentioned this issue Feb 4, 2022
Merged
FIGBERT added a commit to smmr-software/mabel that referenced this issue Feb 20, 2022
@FIGBERT
Update dependencies to fix excess logging
9a72ffa 
anacrolix/go-libutp, a dependency of anacrolix/torrent, did not respect
the logger we configured for our torrent client and would log over the
Bubble Tea UI and caused errors in rendering for the rest of the session
until that section of the screen was refreshed.

We fixed these issues upstream, and this commit incorporates these
changes into Mabel.

Links:
  anacrolix/torrent#676
  anacrolix/go-libutp#19
  anacrolix/torrent#722
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Pass client logger to anacrolix/go-libutp sockets
2 participants
@anacrolix @FIGBERT