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

Remove the build with sanity check method #29

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Conversation

snoyberg
Copy link
Member

Something we discussed offline. The sanity checks will often cause false failures if the primary node is temporarily unavailable. (I just saw this occur for a production service.)

Very much open for discussion on this one, not sure it's the right approach.

Something we discussed offline. The sanity checks will often cause false failures if the primary node is temporarily unavailable. (I just saw this occur for a production service.)
@snoyberg snoyberg requested a review from psibi June 19, 2024 11:25
Copy link
Member

@psibi psibi left a comment

Choose a reason for hiding this comment

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

I'm not sure about this: I think this is useful if we use wrong grpc url etc. That being said, we can probably give this a try and see how it goes.

@snoyberg snoyberg merged commit fc888a6 into main Jun 20, 2024
1 check passed
@snoyberg snoyberg deleted the remove-sanity-check branch June 20, 2024 03:52
@snoyberg
Copy link
Member Author

Cool, let's keep an eye out. Another possibility is adding an explicit sanity_check method, or deferring the chain ID check to the first time we try to do something. Let's see what kinds of failures we run into in practice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants