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

[Merged by Bors] - Do not log slot clock error prior to genesis #4657

Closed

Conversation

jimmygchen
Copy link
Member

Issue Addressed

#4654

Proposed Changes

Only log error if we're unable to read slot clock after genesis.

I thought about simply down grading the error to a warn, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.

@jimmygchen jimmygchen added ready-for-review The code is ready for review UX-and-logs v4.4.1 ETA August 2023 labels Aug 24, 2023
Copy link
Member

@michaelsproul michaelsproul left a comment

Choose a reason for hiding this comment

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

Looks good to me. Maybe @AgeManning or @paulhauner could double-check

Copy link
Member

@paulhauner paulhauner left a comment

Choose a reason for hiding this comment

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

LGTM!

@paulhauner paulhauner added ready-for-merge This PR is ready to merge. and removed ready-for-review The code is ready for review labels Aug 24, 2023
@michaelsproul
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 25, 2023
## Issue Addressed

#4654 

## Proposed Changes

Only log error if we're unable to read slot clock after genesis. 

I thought about simply down grading the `error` to a `warn`, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.
@bors
Copy link

bors bot commented Aug 25, 2023

Build failed (retrying...):

bors bot pushed a commit that referenced this pull request Aug 25, 2023
## Issue Addressed

#4654 

## Proposed Changes

Only log error if we're unable to read slot clock after genesis. 

I thought about simply down grading the `error` to a `warn`, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.
@bors
Copy link

bors bot commented Aug 25, 2023

Build failed:

@michaelsproul
Copy link
Member

bors retry

bors bot pushed a commit that referenced this pull request Aug 25, 2023
## Issue Addressed

#4654 

## Proposed Changes

Only log error if we're unable to read slot clock after genesis. 

I thought about simply down grading the `error` to a `warn`, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.
@bors
Copy link

bors bot commented Aug 25, 2023

Build failed (retrying...):

@michaelsproul
Copy link
Member

bors r-

until we fix the web3signer issue this isn't going to work

@bors
Copy link

bors bot commented Aug 25, 2023

Canceled.

@paulhauner
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Aug 28, 2023
## Issue Addressed

#4654 

## Proposed Changes

Only log error if we're unable to read slot clock after genesis. 

I thought about simply down grading the `error` to a `warn`, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.
@bors
Copy link

bors bot commented Aug 28, 2023

@bors bors bot changed the title Do not log slot clock error prior to genesis [Merged by Bors] - Do not log slot clock error prior to genesis Aug 28, 2023
@bors bors bot closed this Aug 28, 2023
jxs pushed a commit to jxs/lighthouse that referenced this pull request Aug 28, 2023
## Issue Addressed

sigp#4654 

## Proposed Changes

Only log error if we're unable to read slot clock after genesis. 

I thought about simply down grading the `error` to a `warn`, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.
@jimmygchen jimmygchen deleted the remove-error-before-genesis branch September 14, 2023 05:05
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

sigp#4654 

## Proposed Changes

Only log error if we're unable to read slot clock after genesis. 

I thought about simply down grading the `error` to a `warn`, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.
Woodpile37 pushed a commit to Woodpile37/lighthouse that referenced this pull request Jan 6, 2024
## Issue Addressed

sigp#4654 

## Proposed Changes

Only log error if we're unable to read slot clock after genesis. 

I thought about simply down grading the `error` to a `warn`, but feel like it's still unnecessary noise before genesis, and it would be good to retain error log if we're pass genesis. But I'd be ok with just downgrading the log level, too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to merge. UX-and-logs v4.4.1 ETA August 2023
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants