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

fix: consensus metric #4

Merged
merged 11 commits into from
Nov 13, 2023
Merged

fix: consensus metric #4

merged 11 commits into from
Nov 13, 2023

Conversation

tty47
Copy link

@tty47 tty47 commented Nov 8, 2023

hello team!

Some context about this PR, Torch generates a metric to show how long the chain is running, but sometimes if the service is not available for any reason (for example, when autodevops creates a new env, torch starts before the nodes), it generates an issue.

This PR manages this situation creating a task in the background and checking if the end service is available every 10".

Any feedback is welcome!

Cheers! 🚀

cc: @celestiaorg/devops
closes: https://github.com/celestiaorg/devops/issues/571

Signed-off-by: Jose Ramon Mañes <[email protected]>
@tty47 tty47 changed the title Jose/fix cons metric fix: consensus metric Nov 8, 2023
@tty47 tty47 self-assigned this Nov 8, 2023
@tty47 tty47 added bug Something isn't working fix Fixes labels Nov 8, 2023
Signed-off-by: Jose Ramon Mañes <[email protected]>
@tty47 tty47 changed the title fix: consensus metric WIP - fix: consensus metric Nov 8, 2023
@tty47 tty47 marked this pull request as ready for review November 9, 2023 08:24
@tty47 tty47 changed the title WIP - fix: consensus metric fix: consensus metric Nov 9, 2023
Copy link
Member

@smuu smuu left a comment

Choose a reason for hiding this comment

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

lgtm

pkg/http/server.go Outdated Show resolved Hide resolved
pkg/metrics/metrics.go Outdated Show resolved Hide resolved
@tty47 tty47 requested a review from Bidon15 November 9, 2023 11:42
Copy link
Member

@Bidon15 Bidon15 left a comment

Choose a reason for hiding this comment

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

Nice try! Another round to make it good 🥳

pkg/http/server.go Outdated Show resolved Hide resolved
pkg/http/server.go Outdated Show resolved Hide resolved
Signed-off-by: Jose Ramon Mañes <[email protected]>
Signed-off-by: Jose Ramon Mañes <[email protected]>
Copy link
Member

@Bidon15 Bidon15 left a comment

Choose a reason for hiding this comment

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

1 nit, apart from that lgtm

pkg/http/server.go Outdated Show resolved Hide resolved
@tty47 tty47 requested a review from Bidon15 November 13, 2023 12:13
Copy link
Member

@Bidon15 Bidon15 left a comment

Choose a reason for hiding this comment

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

🚢

@tty47 tty47 merged commit e79df58 into main Nov 13, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fix Fixes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants