-
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
Upgrade test #167
Upgrade test #167
Conversation
02c48de
to
98872c6
Compare
1eb5b61
to
c22fdb6
Compare
c22fdb6
to
f227fa9
Compare
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.
Thanks for creating this PR @slinkydeveloper. The changes look good to me. +1 for merging after the required runtime PRs are merged.
test-utils/src/main/kotlin/dev/restate/e2e/utils/RestateDeployer.kt
Outdated
Show resolved
Hide resolved
...ces/java-services/src/main/java/dev/restate/e2e/services/upgradetest/UpgradeTestService.java
Outdated
Show resolved
Hide resolved
UpgradeTestServiceGrpc.SERVICE_NAME, | ||
AwakeableHolderServiceGrpc.SERVICE_NAME, |
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 am wondering whether we shouldn't implement the services for the general tests with the typescript SDK if possible. That way we don't further cement our dependency on the Java SDK.
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.
Fair enough. Do you think should I go and re implement this test in TS or is this comment more something for future tests?
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.
Let's try to do it for new tests from now on, I'd say.
Fix restatedev/restate#504