Skip to content
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

fix(cosmosdb): Add support for the CosmosDB Emulator #579

Merged
merged 15 commits into from
Jun 28, 2024

Conversation

mbenabda
Copy link
Contributor

@mbenabda mbenabda commented May 24, 2024

Adds support for the CosmosDB Emulator container

@mbenabda mbenabda force-pushed the pr/cosmosdb_emulator branch 3 times, most recently from e333a94 to 139eb71 Compare May 24, 2024 20:28
@mbenabda mbenabda marked this pull request as ready for review May 24, 2024 21:31
@alexanderankin alexanderankin added the community-feat feature but its a community module so we wont bump tc core for it label May 24, 2024
@alexanderankin alexanderankin changed the title feat(cosmosdb): Add support for the CosmosDB Emulator fix(cosmosdb): Add support for the CosmosDB Emulator May 24, 2024
@alexanderankin
Copy link
Collaborator

since we do not count community features towards testcontainers features, i have tagged this PR with feat-community but renamed the PR to "fix" - i have fixed the docs build for you but i am not going to get the module tests to pass, they seemed to not pass on the last run

@alexanderankin
Copy link
Collaborator

you can run the tests locally with poetry install -E cosmosdb --with dev && poetry run pytest -s modules/cosmosdb/tests in case it helps you. i was able to validate that the waiting you are doing is in fact working - the log, the wait for url, etc, I suspect the emulator just needs more than two minutes to startup and is timing out? not sure.

@mbenabda
Copy link
Contributor Author

mbenabda commented May 25, 2024

thank you @alexanderankin for looking into this.

Indeed, the container unfortunately takes a while to start up, and I believe you're correct: this is timeout-related failure.

locally, the test passes, and runs consistently in ~1.46s in avg (avg calculated over 100 successive test runs. all passed)

$ time pytest -s modules/cosmosdb/tests
=========================================================================================== test session starts ============================================================================================
platform linux -- Python 3.10.12, pytest-7.4.3, pluggy-1.4.0
rootdir: /tmp/tmp.olcVZbq8gB/testcontainers-python
configfile: pyproject.toml
plugins: asyncio-0.23.5, anyio-4.3.0, cov-4.1.0
asyncio: mode=strict
collected 1 item                                                                                                                                                                                           

modules/cosmosdb/tests/test_cosmosdb.py::test_docker_run Pulling image testcontainers/ryuk:0.7.0

---------------------------------------------------------------------------------------------- live log call -----------------------------------------------------------------------------------------------
INFO     testcontainers.core.container:container.py:88 Pulling image testcontainers/ryuk:0.7.0
Container started: 958785fad0f5
INFO     testcontainers.core.container:container.py:101 Container started: 958785fad0f5
Waiting for container <Container: 958785fad0f5> with image testcontainers/ryuk:0.7.0 to be ready ...
INFO     testcontainers.core.waiting_utils:waiting_utils.py:52 Waiting for container <Container: 958785fad0f5> with image testcontainers/ryuk:0.7.0 to be ready ...
Pulling image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest
INFO     testcontainers.core.container:container.py:88 Pulling image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest
Container started: 2435177c5b4e
INFO     testcontainers.core.container:container.py:101 Container started: 2435177c5b4e
Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...
INFO     testcontainers.core.waiting_utils:waiting_utils.py:52 Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...
Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...
INFO     testcontainers.core.waiting_utils:waiting_utils.py:52 Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...
Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...
INFO     testcontainers.core.waiting_utils:waiting_utils.py:52 Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...
Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...
INFO     testcontainers.core.waiting_utils:waiting_utils.py:52 Waiting for container <Container: 2435177c5b4e> with image mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:latest to be ready ...

(...)

================================================================================ 1 passed, 2 warnings in 105.71s (0:01:45) =================================================================================

real	1m45,927s
user	0m0,875s
sys	0m0,079s

@mbenabda
Copy link
Contributor Author

mbenabda commented May 25, 2024

@alexanderankin could we move this PR back to a draft ? it'll take more time than i expected to have something proper:

  • The emulator container doesn't support port mapping (known limitation)

  • I'm binding port 8081 to the host to check for readiness of the emulator's endpoints
    => there can't be 2 running containers at any given time

  • Tests for != python versions seem to be running on the same host
    => tests are bound to fail

I plan on removing the endpoint readiness check from the module, and document how one would proceed to check for that.

Sorry for the inconvenience this has been ...

@alexanderankin
Copy link
Collaborator

no worries, if you can get the tests to pass we can merge as is and make changes later as well.

i noticed the port limitation myself so i wouldn't worry about changing that

if you can change the timeout configuration during tests so that the tests do actually pass - this is probably the most critical thing - as it is demonstrated in #578, #532

@alexanderankin alexanderankin marked this pull request as draft May 25, 2024 18:04
@mbenabda mbenabda force-pushed the pr/cosmosdb_emulator branch 3 times, most recently from 6f1be19 to 500bc53 Compare May 25, 2024 21:33
@mbenabda
Copy link
Contributor Author

mbenabda commented May 26, 2024

@alexanderankin Hi, i'd like to try running the tests workflows with this. is it possible ? thank you

@alexanderankin
Copy link
Collaborator

looks like "failing after 10m" still taking a while

…rts, and tests are run concurrently for several python versions)
@mbenabda
Copy link
Contributor Author

mbenabda commented May 27, 2024

@alexanderankin looks like we're good now:
https://github.com/mbenabda/testcontainers-python/actions/runs/9254875065

I missed the part where embeded code examples were actually executed as part of the CI

I had to skip the examples from this module from being executed
because to have them run successfully in CI, the examples would have to be changed in a way that doesn't reflect the actual module usage (ie: add bind_ports=False to the container constructors parameters)

unrelated question: shall I squash my commits ?

Thanks

@kiview
Copy link
Member

kiview commented May 27, 2024

What does this mean?

The emulator container doesn't support port mapping (known limitation)

Wondering, since I am not aware of any limitation in this regard from our Java implementation.

@mbenabda
Copy link
Contributor Author

mbenabda commented May 28, 2024

@kiview

What does this mean?

The emulator container doesn't support port mapping (known limitation)

Wondering, since I am not aware of any limitation in this regard from our Java [implementation]>(https://github.com/testcontainers/testcontainers-java/blob/main/modules/azure/src/main/java/org/testcontainers/containers/CosmosDBEmulatorContainer.java).

Now that you mention it, it sounds like BS.

To be honest I read that on a couple of occasions and did not bother to check 👎
I'll look into it and push changes to remove the static port bindings if that pans out

Thank you for pointing that out

@alexanderankin
Copy link
Collaborator

squashing the commits is not necessary. i am really not sure how it works in java because when i was stepping through the python driver, it had some abstractions around determining this port and it was not clear how to inform the azure cosmos driver that it should be using a different port (from the standpoint of my debugging - perhaps if i started from the docs instead it would have been clear)

@eddumelendez
Copy link
Member

cosmosdb client can use two modes: direct and gateway. gateway works in java with random ports and direct is not supported. See testcontainers/testcontainers-java#5518

@saikotek
Copy link
Contributor

saikotek commented Jun 18, 2024

Hello, when it will be added? I too implemented comsosdb emulator support but @mbenabda did it better.

squashing the commits is not necessary. i am really not sure how it works in java because when i was stepping through the python driver, it had some abstractions around determining this port and it was not clear how to inform the azure cosmos driver that it should be using a different port (from the standpoint of my debugging - perhaps if i started from the docs instead it would have been clear)

let's be realistic, this emulator kinda sucks and is heavily restricted. I had the problem with binding ports other than 8081 not because of emulator, but because of python's cosmosdb client ignoring my port configuration. Also official documentation publishes ports 10250-10255 and they do not even say what they are for nor if they are needed..
https://learn.microsoft.com/en-us/azure/cosmos-db/how-to-develop-emulator?tabs=docker-linux%2Cpython&pivots=api-nosql#start-the-emulator
btw I'd suggest squashing the commits to keep the main branch clean.

@alexanderankin
Copy link
Collaborator

should i just clean up and attempt to merge what is in here?

@mbenabda
Copy link
Contributor Author

mbenabda commented Jun 18, 2024

@saikotek @alexanderankin Sorry, I didn't find the time to get back to this these past couple of weeks.

@alexanderankin feel free clean up and merge, it should work with the caveat of binding static ports.

I'll get back to this as soon as things calm down on my side (hopefully starting from next week), and if you've merged by then, open another PR to improve on this basis.

@alexanderankin
Copy link
Collaborator

present pr - main...a85a9b6

rebased - main...pr/cosmosdb_emulator.tmp
rebased permalink - main...b650634

rebased main - main...pr/cosmosdb_emulator.tmp2
rebased main permalink - main...fd04554

@alexanderankin alexanderankin marked this pull request as ready for review June 28, 2024 08:32
Copy link

codecov bot commented Jun 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@e575b28). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #579   +/-   ##
=======================================
  Coverage        ?   76.26%           
=======================================
  Files           ?       11           
  Lines           ?      573           
  Branches        ?       83           
=======================================
  Hits            ?      437           
  Misses          ?      110           
  Partials        ?       26           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alexanderankin alexanderankin force-pushed the pr/cosmosdb_emulator branch 2 times, most recently from 6fc5b9a to c90b36f Compare June 28, 2024 08:46
@alexanderankin alexanderankin merged commit 8045a80 into testcontainers:main Jun 28, 2024
15 checks passed
alexanderankin pushed a commit that referenced this pull request Jun 28, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.7.0](testcontainers-v4.6.0...testcontainers-v4.7.0)
(2024-06-28)


### Features

* **core:** Added Generic module
([#612](#612))
([e575b28](e575b28))
* **core:** allow custom dockerfile path for image build and bypassing
build cache
([#615](#615))
([ead0f79](ead0f79)),
closes
[#610](#610)
* **core:** DockerCompose.stop now stops only services that it starts
(does not stop the other services)
([#620](#620))
([e711800](e711800))


### Bug Fixes

* **ollama:** Add support for ollama module
([#618](#618))
([5442d05](5442d05))
* **cosmosdb:** Add support for the CosmosDB Emulator
([#579](#579))
([8045a80](8045a80))
* improve ollama docs, s/ollama_dir/ollama_home/g
([#619](#619))
([27f2a6b](27f2a6b))
* **kafka:** Add Kraft to Kafka containers
([#611](#611))
([762d2a2](762d2a2))


### Documentation

* **contributing:** add contribution and new-container guide
([#460](#460))
([3519f4b](3519f4b))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
alexanderankin added a commit that referenced this pull request Jul 2, 2024
fixes some incorrectly resolved conflicts during the rebase in #579

fixes #632 

- **fix #607 - no longer need to manually include for toctree**
- **semver note**
- **fix rest of incorrectly resolved conflicts in index.rst**
alexanderankin pushed a commit that referenced this pull request Jul 2, 2024
🤖 I have created a release *beep* *boop*
---


##
[4.7.1](testcontainers-v4.7.0...testcontainers-v4.7.1)
(2024-07-02)


### Bug Fixes

* **core:** bad rebase from
[#579](#579)
([#635](#635))
([4766e48](4766e48))
* **modules:** Mailpit Container
([#625](#625))
([0b866ff](0b866ff))
* **modules:** SFTP Server Container
([#629](#629))
([2e7dbf1](2e7dbf1))
* **network:** Now able to use Network without context, and has labels
to be automatically cleaned up
([#627](#627))
([#630](#630))
([e93bc29](e93bc29))
* **postgres:** get_connection_url(driver=None) should return
postgres://...
([#588](#588))
([01d6c18](01d6c18)),
closes
[#587](#587)
* update test module import
([#623](#623))
([16f6ca4](16f6ca4))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-feat feature but its a community module so we wont bump tc core for it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants