-
Notifications
You must be signed in to change notification settings - Fork 3
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
Wait for service availability at the ingress before running tests #267
Conversation
@@ -1,5 +1,5 @@ | |||
# Timeout config | |||
junit.jupiter.execution.timeout.testable.method.default=10 s | |||
junit.jupiter.execution.timeout.testable.method.default=15 s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This timeout has been increased by 5 seconds because it can take up to 5s until the new schema information has been propagated from the schema registry to the ingress.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is gonna kill the execution time of tests... Could we have a way to significantly lower down the polling time, to say 100ms, for tests just by configuring an env variable here https://github.com/restatedev/e2e/blob/main/tests/build.gradle.kts#L51?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also isn't 5 seconds generally too much? Also for the local dev experience...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a commit which eagerly pushes schema information changes to the worker. This will make the schema information schema propagation appear almost instantaneously. Will revert this change then.
This commit makes the RestateDeployer wait for the availability of registered services at the ingress. The deployer checks whether the ingress is serving the registered services via calling grpc.health.v1.Health/Check and checking that the response contains status = SERVING. This fixes restatedev#266.
49edea2
to
551fe3d
Compare
ping @slinkydeveloper |
Is this still relevant? |
I think technically yes once we go distributed. Right now it just happens to not be needed because of the single process setup. I guess we no longer have the health service which we can call to check for the registered services. Is there another way at the Ingress to figure out whether a service is known with the new ingress? |
|
Closing since the PR is currently not needed and quite out of sync with the current code base. |
This commit makes the RestateDeployer wait for the availability of registered
services at the ingress. The deployer checks whether the ingress is serving
the registered services via calling grpc.health.v1.Health/Check and checking
that the response contains status = SERVING.
This fixes #266.