Skip to content

Conversation

samt1803
Copy link
Contributor

@samt1803 samt1803 commented Aug 28, 2023

Copy link
Contributor

@jtakalai jtakalai left a comment

Choose a reason for hiding this comment

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

should it have a health check? I wrote a quick attempt at something like that, though really it's kind of hard to know if it REALLY is healthy before there is traffic -___- see #617

Copy link
Contributor

@harbu harbu left a comment

Choose a reason for hiding this comment

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

Is this ETH-568?

We should add some kind of health check at least. Adding a few comments to what @jtakalai is working on in streamr-dev/network-contracts#617. Could we add that here? The file writing approach there is better than having no health check.

@samt1803
Copy link
Contributor Author

Ah I didn't see that ticket. It is that, together with #615, that needs this to be merged to get green in CI and now #617

@jtakalai
Copy link
Contributor

jtakalai commented Sep 12, 2023

Could we add that here?

I think since it has its own PR, we could just leave it out from here. Let's get this one merged.

@harbu
Copy link
Contributor

harbu commented Sep 12, 2023

Having the health check here is kind of important if we want to re-enable the skipped tests in client related to this service (mainly https://github.com/streamr-dev/network/blob/streamr-1.0/packages/client/test/end-to-end/StreamRegistry.test.ts#L116). Unless we have another way to verify that the service is running before starting the test run?

@samt1803
Copy link
Contributor Author

added healthcheck to container

@samt1803 samt1803 merged commit 331c113 into master Sep 15, 2023
@samt1803 samt1803 deleted the ens_sync_fastchain branch September 15, 2023 13:13
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.

4 participants