Skip to content

Conversation

lidel
Copy link
Member

@lidel lidel commented Jun 6, 2025

Arguably, these are not low-level details from DEBUG level of individual CIDs, but high level INFO about start/stop of reprovide.

@lidel lidel requested a review from a team as a code owner June 6, 2025 14:39
start := time.Now()
err := doProvideMany(s.ctx, s.rsys, keys)
if err != nil {
log.Debugf("reproviding failed %v", err)
log.Infof("reproviding failed %v", err)
Copy link
Member Author

Choose a reason for hiding this comment

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

💭 Should this be an error?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, an error is only returned if the DHT provider was unable to provide any record.

Copy link
Contributor

@guillaumemichel guillaumemichel left a comment

Choose a reason for hiding this comment

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

This change makes sense.

start := time.Now()
err := doProvideMany(s.ctx, s.rsys, keys)
if err != nil {
log.Debugf("reproviding failed %v", err)
log.Infof("reproviding failed %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, an error is only returned if the DHT provider was unable to provide any record.

@hsanjuan
Copy link
Contributor

Triage:

  • Consider adding additional debug statements for each cid provided etc.
  • Consolidate provider logging facilities into a single provider (current provider.simple, provider.batched etc...)

@hsanjuan hsanjuan self-assigned this Jun 10, 2025
Copy link
Contributor

@hsanjuan hsanjuan left a comment

Choose a reason for hiding this comment

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

I have added some more logging and consolidated logging facilities into provider.

lidel and others added 4 commits June 13, 2025 13:05
@hsanjuan hsanjuan force-pushed the chore-adjust-provider-log-level branch from f43625c to 6cd7e50 Compare June 13, 2025 11:05
@gammazero gammazero merged commit ba8d41a into main Jun 16, 2025
18 of 20 checks passed
@gammazero gammazero deleted the chore-adjust-provider-log-level branch June 16, 2025 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants