Adding support for Cassandra and Scylla

[fruch]

This add the support for those cassandra based dbs and their drivers, cassandra-driver, scylla-driver

Ref: https://cassandra.apache.org/ Ref: https://www.scylladb.com/ Ref: https://pypi.org/project/cassandra-driver/ Ref: https://pypi.org/project/scylla-driver/

[fruch]

Hi @tillahoffmann and @SergeyPirogov, pretty please for approving the workflow to start ?

FYI, it can be eased a bit, the approval part: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/enabling-features-for-your-repository/managing-github-actions-settings-for-a-repository#configuring-required-approval-for-workflows-from-public-forks

[yakimka]

@fruch hello. Can you update this PR?

[tillahoffmann]

Thanks for the additions and apologies for the slow reply. A few things to get this integrated:

• Rebase on master.
• Regenerate requirement files.
• Add cassandra and scylla to the test-components in .github/workflows/main.yml.

[fruch]

Thanks for the additions and apologies for the slow reply. A few things to get this integrated:

• Rebase on master.
• Regenerate requirement files.
• Add cassandra and scylla to the test-components in .github/workflows/main.yml.

I've rebased and added the test to CI

but the building of requirements is failing:

There are incompatible versions in the resolved dependencies:
  protobuf<4.0.0dev (from google-cloud-pubsub==1.7.2->testcontainers===dev->-r requirements.in (line 1))
  protobuf<5.0.0dev,>=3.20.1 (from google-api-core[grpc]==2.10.1->google-cloud-pubsub==1.7.2->testcontainers===dev->-r requirements.in (line 1))
  protobuf<5.0.0dev,>=3.15.0 (from googleapis-common-protos[grpc]==1.56.4->google-api-core[grpc]==2.10.1->google-cloud-pubsub==1.7.2->testcontainers===dev->-r requirements.in (line 1))
  protobuf>=4.21.3 (from grpcio-status==1.49.0->google-api-core[grpc]==2.10.1->google-cloud-pubsub==1.7.2->testcontainers===dev->-r requirements.in (line 1))

I think pinning of google-cloud-pubsub is problematic, and google packages are conflicting each other. I'm trying to unpin it, and regenerate (but it quite a long process, meanwhile it worked for python3.7)

[fruch]

@yakimka @tillahoffmann waiting for approval for running the CI

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

:exclamation: No coverage uploaded for pull request base (main@ca65a91). Click here to learn what that means.

:exclamation: Current head dd6520d differs from pull request most recent head 1114f4d. Consider uploading reports for the commit 1114f4d to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #167   +/-   ##
=======================================
  Coverage        ?   87.81%           
=======================================
  Files           ?       32           
  Lines           ?      886           
  Branches        ?       58           
=======================================
  Hits            ?      778           
  Misses          ?       78           
  Partials        ?       30           

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[tillahoffmann]

We may have to get #209 merged first to get around the google dependency issue.

[tillahoffmann]

~I have a relatively strong preference to remove the cache for two reasons. First, it's more code to maintain. Second, I don't think the caching will help in most real-world scenarios.~

~I appreciate that runtime is different across different machines, but getting a single image takes about 4ms on my machine. Using the cache, getting a single image takes about 300ms. It'll of course be quicker the next time images are fetched, but we'd have to use at least 75 containers per test suite to make the cache worthwhile. To have a delay of one second over the cached version, we'd have to run about 325 containers. I'd argue that for a test suite that runs more than 300 docker containers a delay of one second is negligible. But let me know if I misunderstood/miscalculated.~

[fruch]

We may have to get #209 merged first to get around the google dependency issue.

Seems that just removing the pinning helped

[fruch]

I have a relatively strong preference to remove the cache for two reasons. First, it's more code to maintain. Second, I don't think the caching will help in most real-world scenarios.

I appreciate that runtime is different across different machines, but getting a single image takes about 4ms on my machine. Using the cache, getting a single image takes about 300ms. It'll of course be quicker the next time images are fetched, but we'd have to use at least 75 containers per test suite to make the cache worthwhile. To have a delay of one second over the cached version, we'd have to run about 325 containers. I'd argue that for a test suite that runs more than 300 docker containers a delay of one second is negligible. But let me know if I misunderstood/miscalculated.

Didn't understand where you are going with the calculation here, for me getting a docker image out of dockerhub can take a minute or two. I was more talking about pip-compile that in case of conflicts can download all the versions of a package to try solving it, (I know it, cause I'm using it in in one of my projects in my day job),

And as you said I don't think it's worth while for a library to maintain such a lock file. even that tool like poetry for example are doing just that. (also can be slow and annoying)

[tillahoffmann]

Sorry, I posted on the wrong thread. 🤦‍♂️

[fruch]

We may have to get #209 merged first to get around the google dependency issue.

@tillahoffmann would it be o.k. to take the code changes from #209, and test it in tandam ?

[spicy-sauce]

Hi, What's the status of this PR? Do you plan to merge it to the master anytime soon?

@fruch I see there are conflicts that should be resolved, is it something that you should handle?

Thanks, Yossi

[fruch]

Hi, What's the status of this PR? Do you plan to merge it to the master anytime soon?

@fruch I see there are conflicts that should be resolved, is it something that you should handle?

I guess I could, I've almost forgotten about this one, it's almost a whole year since I've opened this one.

Thanks, Yossi

[spicy-sauce]

I guess I could, I've almost forgotten about this one, it's almost a whole year since I've opened this one.

Thanks!

@tillahoffmann can we please push it once @fruch resolves the conflicts? This seems like a great addition to testcontainers!

[fruch]

I guess I could, I've almost forgotten about this one, it's almost a whole year since I've opened this one.

Thanks!

@tillahoffmann can we please push it once @fruch resolves the conflicts? This seems like a great addition to testcontainers!

BTW @spicy-sauce can you share a bit what do you need it for ? Cassandra or Scylla ? And which type of testing it's gonna be part of ?

[spicy-sauce]

BTW @spicy-sauce can you share a bit what do you need it for ? Cassandra or Scylla ? And which type of testing it's gonna be part of ?

At this point I need it for Cassandra as I'm going to implement such a block in DataYoga and would love to use it for the testing part.

Read more here: https://datayoga-io.github.io/datayoga/

[fruch]

BTW @spicy-sauce can you share a bit what do you need it for ? Cassandra or Scylla ? And which type of testing it's gonna be part of ?

At this point I need it for Cassandra as I'm going to implement such a block in DataYoga and would love to use it for the testing part.

Read more here: https://datayoga-io.github.io/datayoga/

looks interesting, ping me when you have it running with Cassandra or Scylla, maybe adding it in our https://university.scylladb.com/

anyhow I'm the maintainer of python driver fork https://github.com/scylladb/python-driver

if you have question you can reach me at [email protected]

[fruch]

@tillahoffmann #209 didn't seemed to be merged

Anyhow I've rebased again, and rebuild the requirements files again.

Can now it be run in CI and merged ?

[fruch]

@tillahoffmann looks like I've missed updating the requriements files for 3.7, now it should run o.k.

[spicy-sauce]

Guys, can we please push this one? It's really a great addition and we would like to avoid another conflicts. Thanks a lot!!!

[fruch]

@tillahoffmann @yakimka it's now passes all the tests...

[spicy-sauce]

@fruch While we are waiting for it, can you please add the new containers to README.rst? @tillahoffmann I see RedisContainer is also missing.

[fruch]

@fruch While we are waiting for it, can you please add the new containers to README.rst? @tillahoffmann I see RedisContainer is also missing.

I first need to fix the doctests, f**king white spaces seems to be breaking it.

[spicy-sauce]

@fruch this one failed: testcontainers-python / build (3.9, cassandra.py)

https://github.com/testcontainers/testcontainers-python/actions/runs/3788545927/jobs/6442220582

Thanks!

[fruch]

@fruch this one failed: testcontainers-python / build (3.9, cassandra.py)

https://github.com/testcontainers/testcontainers-python/actions/runs/3788545927/jobs/6442220582

Thanks!

Well it's a bit hard to debug the issue, since those doctests are not collecting the docker images logs...

Kind of pointless to have the same tests written, and in the doctests, it's just repeating the same tests with any pytest facilities you can write to collect information.

Also I can rerun the action on my own, so can even gather statistics on the issue.

[tillahoffmann]

Kind of pointless to have the same tests written, and in the doctests, it's just repeating the same tests with any pytest facilities you can write to collect information.

The main point of the doctests is to ensure the documentation remains consistent with any changes that might be made to the interface rather than testing the code for correctness.

I've rerun the tests and everything is passing. One more rebase, and we should be able to merge this one. Thank you for the patience.

[fruch]

Kind of pointless to have the same tests written, and in the doctests, it's just repeating the same tests with any pytest facilities you can write to collect information.

The main point of the doctests is to ensure the documentation remains consistent with any changes that might be made to the interface rather than testing the code for correctness.

I've rerun the tests and everything is passing. One more rebase, and we should be able to merge this one. Thank you for the patience.

fixed merge conflicts

[guy9]

Is there anything preventing this from being merged?

[alexanderankin]

Currently it doesn't make too much of a difference what is merged or not, because there has been no activity in the repository from either individuals with Pypi publish credentials. E.g. see #405. That said next week I think we are going to start the process according to pep 541 to reclaim the Pypi name and start releasing and merging changes again