update dapr runtime to use v1.12.0-rc.1 #906
Conversation
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
|
@cicoyle - We create a release-1.12 branch in this repo. Can you retarget this PR against the new branch, so that we test this independent of master (main) branch. |
Done. I guess we will want to re trigger the workflows again - can someone help me out with that? |
|
Shouldn't the redis version also be pinned here? |
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Signed-off-by: Cassandra Coyle <[email protected]>
Maybe, I pinned it to a version I saw in your PR here. |
Signed-off-by: Cassandra Coyle <[email protected]>
| err = client.UnsubscribeConfigurationItems(context.Background(), DAPR_CONFIGURATION_STORE, subscriptionID) | ||
| if err != nil { | ||
| fmt.Println("Error unsubscribing to config updates, err:" + err.Error()) | ||
| } else { | ||
| fmt.Println("App unsubscribed to config changes") | ||
| } |
There was a problem hiding this comment.
why is this being removed?
Is this removed from SDK or not a valid operation anymore?
There was a problem hiding this comment.
It has been deprecated in runtime https://github.com/dapr/dapr/blob/9105124636459e751964f3a68845fc63ccbb45e9/pkg/grpc/api.go#L1459
It should be marked as such in SDKs (I don't think it is atm anywhere).
There was a problem hiding this comment.
I agree it should be removed ... but is it causing an issue ...
On a side note, @dapr/maintainers-quickstarts When will the latest SDK RCs will be tested for quickstarts.
There was a problem hiding this comment.
I agree it should be removed ... but is it causing an issue ...
Yes, with it included the test runner is printing out an error saying that the subscription cannot be unsubscribed because it doesn't exist. That is a true error- the subscription at the top of the file is given a context with a timeout of 10 seconds, we then do some work, then sleep 10 seconds. This means that when we go to unsubscribe here, the subscription context has been cancelled and closed, and therefore unsubscribed. The unsubscribe here now makes no sense.
The relevant runtime PR: dapr/dapr#6769
There was a problem hiding this comment.
ok that is marked as a breaking change ....
There was a problem hiding this comment.
Also the other question, is the unsubscribe method only failing in GO SDK?
Does it fail in any other SDK?
There was a problem hiding this comment.
@mukundansundar I assume it is not failing in other quickstart tests because the logic is different. For example, JS is doing the right thing here by just closing the stream
There was a problem hiding this comment.
There was a problem hiding this comment.
I believe the context timeout was also causing an issue due to us having a context with a timeout, then sleeping for that duration.
|
Interesting the bindings tutorial seems to be failing consistently. |
Co-authored-by: Josh van Leeuwen <[email protected]> Signed-off-by: Cassie Coyle <[email protected]>
| err = client.UnsubscribeConfigurationItems(context.Background(), DAPR_CONFIGURATION_STORE, subscriptionID) | ||
| if err != nil { | ||
| fmt.Println("Error unsubscribing to config updates, err:" + err.Error()) | ||
| } else { | ||
| fmt.Println("App unsubscribed to config changes") | ||
| } |
| @@ -134,7 +134,7 @@ jobs: | |||
| run: | | |||
| helm repo add bitnami https://charts.bitnami.com/bitnami | |||
| helm repo update | |||
| helm install redis bitnami/redis | |||
| helm install redis bitnami/redis --version 17.14.5 | |||
There was a problem hiding this comment.
Just curious, why do we want to pin? this can catch us later on too when we forget :)
There was a problem hiding this comment.
Per Arturs comment above ^, tho also not sure on the exact reasoning for it
Description
Update dapr runtime to use v1.12.0-rc.3 && dapr cli to use v1.12.0-rc.1 to test & validate the release candidate
Issue reference
dapr/dapr#6899
Checklist