chore: Expand contract tests to support persistent data stores #199
Conversation
keelerm84
force-pushed
the
mk/sdk-753/data-system-tests
branch
from
aae906f to
39b39bd
Compare
keelerm84
changed the title
keelerm84
force-pushed
the
mk/sdk-753/data-system-tests
branch
5 times, most recently
from
02ebdc5 to
fa709be
Compare
keelerm84
force-pushed
the
mk/sdk-753/data-system-tests
branch
from
a3215f0 to
2bf606d
Compare
keelerm84
changed the base branch from
feat/persistent-data-store-tests
to
v7
| dynamodb: | ||
| image: amazon/dynamodb-local | ||
| ports: | ||
| - 8000:8000 |
There was a problem hiding this comment.
Kinda surprised they "took" port 8000 for dynamodb.
| awsconfig.WithRegion("us-east-1"), | ||
| awsconfig.WithCredentialsProvider(credentials.NewStaticCredentialsProvider("dummy", "dummy", "dummy")), | ||
| ) | ||
| if err != nil { |
There was a problem hiding this comment.
The function that calls makeSDKConfig is capable of returning an error, so it would be good to change the signature here so we can bubble this back up
testservice/sdk_client_entity.go
Outdated
| } | ||
|
|
||
| builder = ldcomponents.PersistentDataStore(dsBuilder) | ||
| default: // TODO: We should probably fail here. |
There was a problem hiding this comment.
This could also return an error
| @@ -246,7 +249,7 @@ func main() { | |||
| loggers := ldlog.NewDefaultLoggers() | |||
| loggers.SetMinLevel(logLevelFromName(os.Getenv("LD_LOG_LEVEL"))) | |||
|
|
|||
| port := "8000" | |||
| port := "10000" | |||
There was a problem hiding this comment.
Was changing the dynamo port a no-go, rather than changing the test service's port?
There was a problem hiding this comment.
I could have changed the dynamodb port. But since every other SDK is going to have to override it (and then each of us will when doing local work), I thought it might be better to try and get those ports on the standard ones, and then maybe we can align on an SDK test harness port.
| CapabilityClientPrereqEvents = "client-prereq-events" | ||
| CapabilityPersistentDataStoreRedis = "persistent-data-store-redis" | ||
| CapabilityPersistentDataStoreConsul = "persistent-data-store-consul" | ||
| CapabilityPersistentDataStoreDynamoDB = "persistent-data-store-dynamodb" |
There was a problem hiding this comment.
Not for this PR: I think we really don't need constants for these things.
|
Awesome work! |
No description provided.