HENNGE / aiodynamo

Asynchronous, fast, pythonic DynamoDB Client
https://aiodynamo.readthedocs.io/
Other
69 stars 20 forks source link

Add ScyllaDB Alternator to test suite #151

Closed ojii closed 9 months ago

ojii commented 1 year ago

Adds ScyllaDB Alternator to our suite of implementations to test with.

Changed how the test suite finds local implementations and made it easier to run tests with multiple implementations at once (DYNAMODB_URL -> DYNAMODB_URLS), though DYNAMODB_URL will still work (since it's useful for GHA).

Scylla handles DescribeTable a bit different from other implementations, see https://github.com/scylladb/scylladb/issues/12930, but that's not too bad since it found a bug in describe_table (returning throughput=None instead of throughput=PayPerRequest()), so I've updated the test accordingly, though that requires setting the "flavor" to "scylla" for those tests (either via DYNAMODB_FLAVOR=scylla or DYNAMODB_URLS='scylla=http://endpoint,scylla'

Also used this chance to add 3.11 testing.

ojii commented 1 year ago

Predicatbly, I broke the GHA workflow file so badly it doesn't even run any tests.

dimaqq commented 1 year ago

quite a few changes are mixed in together... from newer python to scylla to code fixes

i guess that's why it's a draft :)

ojii commented 1 year ago

quite a few changes are mixed in together... from newer python to scylla to code fixes

i guess that's why it's a draft :)

It's a draft because I broke Github Actions somehow and Github decides if you break it, that's not "failing tests" but "no tests, everything ok!" 🤦

dimaqq commented 1 year ago

The workflow is not valid. .github/workflows/workflow.yml (Line: 115, Col: 9): Unexpected value 'command'

Perhaps the value can only be a string and not a list?

GHA are opt-in. Set up branch protection rules for the PR target branch and require specific tests to pass to avoid silent failures.

P.S. I too find GHA YAML validation rather unfriendly to devs... I had complained about it too, but to no avail.

FurqanHabibi commented 1 year ago

I wouldn't recommend using GHA service container and this is one of the reason, it does not yet support passing arguments to the container 🤯 . That's why it errored on command, because there's no command option in services.

Instead just use docker-compose and run it in a step, that's what we do in unreadchecker for example.

FurqanHabibi commented 1 year ago

And about the GHA not running any check, yes because the workflow yaml is invalid, so it is not counted as a check.

FurqanHabibi commented 1 year ago

I've setup the docker-composes for the tests in this branch . I can merge it to this branch if you want. Do note that the tests are not passing 100% yet.

ojii commented 1 year ago

I've setup the docker-composes for the tests in this branch . I can merge it to this branch if you want. Do note that the tests are not passing 100% yet.

Thank you 💯

FurqanHabibi commented 1 year ago

@ojii This should be good to merge right?

ojii commented 1 year ago

@ojii This should be good to merge right?

yea if we can fix ci

dimaqq commented 9 months ago

Woo-hoo!!!