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.

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

Use Log instead of Logs #46

Merged
merged 2 commits into from
Oct 10, 2024
Merged

Use Log instead of Logs #46

merged 2 commits into from
Oct 10, 2024

Conversation

reynir
Copy link
Contributor

@reynir reynir commented Oct 10, 2024

I noticed my application was logging happy-eyeballs warnings. This was slightly confusing to me.

@reynir
Copy link
Contributor Author

reynir commented Oct 10, 2024

This gets logged a lot for me in opam-mirror. I'm not sure what it means or why it should be a warning...

@hannesm
Copy link
Contributor

hannesm commented Oct 10, 2024

Are doing a lot of concurrent requests? The log message appears if we just resolved some IPv4 addresses, and add them to the set of addresses to connect to. But we have not yet received an answer from the task querying for AAAA records (IPv6 address records) for a given hostname. And we'll wait a bit longer since we prefer IPv6.

We can demote to Log.info or Log.debug if that makes your life / output easier :)

EDIT: updated the above after reading the code.

@reynir
Copy link
Contributor Author

reynir commented Oct 10, 2024

Yes, I think it defaults to up to 20 concurrent downloads, and a lot of the archives are on the same few hosts (e.g. github)

src/happy_eyeballs.ml Outdated Show resolved Hide resolved
@hannesm
Copy link
Contributor

hannesm commented Oct 10, 2024

let's demote to debug, and include the hostname.

- Demote to debug logging,
- Print as well the host in question

Co-authored-by: Hannes Mehnert <hannes@mehnert.org>
@hannesm hannesm merged commit 1b8e428 into main Oct 10, 2024
3 checks passed
@hannesm hannesm deleted the log-warn branch October 10, 2024 09:48
@hannesm
Copy link
Contributor

hannesm commented Oct 10, 2024

should we do a release?

@reynir
Copy link
Contributor Author

reynir commented Oct 10, 2024

I think that would be nice

hannesm added a commit to hannesm/opam-repository that referenced this pull request Oct 10, 2024
CHANGES:

* Demote log message for Waiting_for_AAAA and Resolve_a (@reynir, robur-coop/happy-eyeballs#46)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants