feat(CosmosDb): Update base image from latest to vnext-preview#1324
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
HofmeisterAn
left a comment
HofmeisterAn
left a comment
There was a problem hiding this comment.
Thanks for creating the PR. Have you seen this discussion? I would like to wait until an official version is published. In addition, we do not use static ports. While GetAvailablePort() does not return a static port per se, it may fail for remote container runtimes (it does not resolve the mapped port from the container host).
For internal routing to work as expected, the API port and mapping port need to be configured with the same value. References.
According to the discussion, this is unnecessary, and it is why we used the DelegatingHandler in the past.
I think we can prepare a PR and merge it as soon as the official version is published, but I favor and would like to implement the proposed changes from the discussion.
@HofmeisterAn Thanks for your comment. I hadn't come across that discussion. I only checked if there were any open issues. I ended up commenting there in the discussion as well. Looking at the discussion, it seems that using the rewriting of the DelegatingHandler is still the best approach, as it also removes the need to know in advance the port where the container will be randomly opened. Even so, some workarounds, like bypassing the certificate, are no longer necessary. |
WojciechNagorski
left a comment
WojciechNagorski
left a comment
There was a problem hiding this comment.
I'm waiting for this PR because currently, it takes forever to start the image :)
You can already use the |
The solution recommended on the discussion (#1306 (comment)) is 100% stable. Sometimes I got internal server error 500. Logs shows the |
HofmeisterAn
added
module
HofmeisterAn
force-pushed
the
develop
branch
3 times, most recently
from
4900ecd to
8fa5f1b
Compare
|
Does anyone know if |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughSwitches emulator image to vnext-preview, exposes health port 8080 and probes /alive, changes container endpoints and URI rewriting from HTTPS to HTTP, adds a dedicated health-check HttpClient/handler with JSON-based readiness gating, and updates tests to create/read items against the emulator. Changes
Sequence DiagramsequenceDiagram
participant Starter as Container Startup
participant Builder as CosmosDbBuilder
participant HealthClient as HttpClient (health)
participant HealthAPI as /alive Endpoint
participant Ready as Readiness Gate
Starter->>Builder: Initialize (bind ports :8080, set ENABLE_EXPLORER=false)
Builder->>HealthClient: create health-check HttpClient/handler
loop Poll /alive until ready
Builder->>HealthClient: GET http://localhost/alive
HealthClient->>HealthAPI: request /alive
HealthAPI-->>HealthClient: 200 + JSON { checks: { gateway: "...", postgres: "..." } }
HealthClient-->>Builder: return JSON
Builder->>Builder: parse checks (gateway & postgres == "healthy"?)
alt both healthy
Builder-->>Ready: mark container ready
else not healthy
Builder->>Builder: wait/retry/backoff
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the Cosmos DB emulator used by the CosmosDb module/tests to Microsoft’s new Linux-based emulator image (vnext-preview) and adjusts the container/test wiring accordingly.
Changes:
- Pin Cosmos DB emulator image to
mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview. - Update CosmosDb container wiring to use HTTP and a new
/alive-based readiness check (plus disable Explorer UI). - Unskip/expand integration tests to validate database/container creation and item CRUD via Gateway mode.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Testcontainers.CosmosDb.Tests/Dockerfile | Updates emulator image tag to vnext-preview. |
| tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs | Enables and expands integration tests against the new emulator. |
| src/Testcontainers.CosmosDb/CosmosDbBuilder.cs | Updates default image tag, adds health-check port binding and /alive wait logic, disables Explorer. |
| src/Testcontainers.CosmosDb/CosmosDbContainer.cs | Switches endpoint/rewrites to HTTP and adds a dedicated health-check HttpClient. |
| src/Testcontainers.CosmosDb/Testcontainers.CosmosDb.csproj | Formatting/newline-only change. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cs (1)
8-8:⚠️ Potential issue | 🟠 MajorAvoid setting a preview image as the default module image.
Line 8 hardcodes
vnext-previewas the default. That makes the public default depend on a preview channel, which can shift behavior unexpectedly for downstream users and CI. Prefer a stable/official default, with preview as explicit opt-in.Is `mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview` still a preview-only tag, and what is the currently recommended stable GA tag for the Linux Cosmos DB emulator?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs` at line 8, The constant CosmosDbImage currently hardcodes the preview tag; change the public default so it uses the stable/GA image (replace the value of CosmosDbImage with the recommended GA tag) and provide a separate constant (e.g., CosmosDbPreviewImage) or configuration flag to opt into "mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview" for preview testing; update any code that references CosmosDbImage to use the new default and allow selecting the preview constant explicitly. Ensure the new default value is verified against the current official GA tag before committing.
🧹 Nitpick comments (1)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cs (1)
162-164: Avoid swallowing readiness errors silently.The empty catch makes startup failures difficult to diagnose in CI. Consider at least debug-level logging of the exception message before returning
false.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs` around lines 162 - 164, The empty catch block in CosmosDbBuilder (the catch immediately before the method's final "return false") swallows exceptions and hinders CI debugging; change it to catch (Exception ex) and emit a debug-level log of ex.Message/exception (using the class's existing logger field such as _logger/Logger/Log or add an ILogger if none exists) before returning false so readiness errors are recorded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Testcontainers.CosmosDb/CosmosDbContainer.cs`:
- Around line 29-30: The XML summary is outdated: update the documentation for
the method/property that returns the HTTP client (e.g.,
GetHttpClient/CreateHttpClient/HttpClient in CosmosDbContainer) to state that
the client routes over HTTP and does not configure or bypass certificate
validation (remove any claim that it "trusts the emulator certificate"); also
update any related remarks/tags to reflect current behavior and remove
references to certificate validation or TLS trusts.
- Around line 31-37: The properties HttpClient and HttpClientHealthCheck
currently create new UriRewriter and HttpClient on every access; instead add a
single field-level UriRewriter (e.g., _uriRewriter and _uriRewriterHealth)
inside CosmosDbContainer (or a static cached instance) and change the properties
to construct HttpClient via new HttpClient(_uriRewriter, disposeHandler: false)
and new HttpClient(_uriRewriterHealth, disposeHandler: false) (or expose an
explicit factory method GetHttpClient(Func<UriRewriter, HttpClient>) that
documents ownership), ensuring the UriRewriter instances are reused and not
disposed by CosmosClient; reference the existing symbols HttpClient,
HttpClientHealthCheck, UriRewriter, CosmosDbContainer, and
CosmosDbBuilder.CosmosDbPort/CosmosDbHealthCheckPort when making the change.
In `@tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs`:
- Around line 36-42: The test reads databaseProperties via an unfiltered query
which is non-deterministic; instead, fetch the created DB by name: after using
cosmosClient.CreateDatabaseIfNotExistsAsync("fakedb") and creating the
container, call cosmosClient.GetDatabase("fakedb").ReadAsync() (or the sync
equivalent) to obtain the DatabaseProperties and then Assert.Equal("fakedb",
databaseProperties.Id); replace usage of
GetDatabaseQueryIterator<DatabaseProperties>().ReadNextAsync().First() with this
direct ReadAsync call on cosmosClient.GetDatabase("fakedb") to make the
assertion deterministic.
---
Duplicate comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Line 8: The constant CosmosDbImage currently hardcodes the preview tag; change
the public default so it uses the stable/GA image (replace the value of
CosmosDbImage with the recommended GA tag) and provide a separate constant
(e.g., CosmosDbPreviewImage) or configuration flag to opt into
"mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview" for
preview testing; update any code that references CosmosDbImage to use the new
default and allow selecting the preview constant explicitly. Ensure the new
default value is verified against the current official GA tag before committing.
---
Nitpick comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Around line 162-164: The empty catch block in CosmosDbBuilder (the catch
immediately before the method's final "return false") swallows exceptions and
hinders CI debugging; change it to catch (Exception ex) and emit a debug-level
log of ex.Message/exception (using the class's existing logger field such as
_logger/Logger/Log or add an ILogger if none exists) before returning false so
readiness errors are recorded.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cssrc/Testcontainers.CosmosDb/CosmosDbContainer.cssrc/Testcontainers.CosmosDb/Testcontainers.CosmosDb.csprojtests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cstests/Testcontainers.CosmosDb.Tests/Dockerfile
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cs (1)
163-163: Consider narrowing the exception handling or adding a brief comment.The empty catch block silently swallows all exceptions. While this is acceptable for wait strategies (where failures mean "not ready, retry"), catching
Exceptioncould hide unexpected issues during development.💡 Optional: Add clarity with targeted exception handling
- catch { } + catch (Exception) + { + // Any failure (network, timeout, JSON parsing) means the container is not ready yet. + // Return false to trigger a retry. + }Alternatively, consider catching more specific exceptions like
HttpRequestExceptionandJsonExceptionif you want to surface truly unexpected errors.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs` at line 163, The empty catch in CosmosDbBuilder (inside the wait/retry probe block) currently swallows all exceptions; change it to catch specific exceptions (e.g., HttpRequestException, JsonException, TaskCanceledException) or rethrow unexpected ones, and add a short comment explaining that transient probe failures are ignored; refer to the catch in the CosmosDbBuilder class (the wait/ready probe TRY/CATCH) and update it to either list those exception types or log/throw for non-transient errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Line 113: The health-check currently calls
((CosmosDbContainer)container).HttpClientHealthCheck inside a using, which
forces creation and disposal of a new HttpClient each access
(HttpClientHealthCheck is a getter that new's one), defeating pooling and
risking socket exhaustion; fix by either changing
CosmosDbContainer.HttpClientHealthCheck to return a single reusable HttpClient
instance (cache a private readonly HttpClient and return it) or, if the
container already manages lifetime, remove the using and stop disposing it in
CosmosDbBuilder.cs so the same HttpClient is reused across health checks (ensure
you call the container’s Dispose appropriately elsewhere).
---
Nitpick comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Line 163: The empty catch in CosmosDbBuilder (inside the wait/retry probe
block) currently swallows all exceptions; change it to catch specific exceptions
(e.g., HttpRequestException, JsonException, TaskCanceledException) or rethrow
unexpected ones, and add a short comment explaining that transient probe
failures are ignored; refer to the catch in the CosmosDbBuilder class (the
wait/ready probe TRY/CATCH) and update it to either list those exception types
or log/throw for non-transient errors.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs (1)
36-39:⚠️ Potential issue | 🟡 MinorUse deterministic database read for the assertion source.
Using
First()from an unfiltered listing can be order-dependent. Read the created database directly.✅ Proposed fix
- var databaseProperties = (await cosmosClient.GetDatabaseQueryIterator<DatabaseProperties>().ReadNextAsync()).First(); + var databaseProperties = (await database.ReadAsync()).Resource;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs` around lines 36 - 39, The test currently obtains the database for assertion by enumerating databases via cosmosClient.GetDatabaseQueryIterator<DatabaseProperties>().ReadNextAsync() and calling First(), which is order-dependent; instead, use the deterministic API to read the created database directly by calling cosmosClient.GetDatabase("fakedb") and invoking ReadAsync/ReadDatabaseAsync (or the equivalent synchronous read) on that Database/DatabaseProperties handle returned after CreateDatabaseIfNotExistsAsync; update the assertion source to use the result of CreateDatabaseIfNotExistsAsync or the direct read of cosmosClient.GetDatabase("fakedb") rather than querying and calling First().
🧹 Nitpick comments (1)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cs (1)
163-163: Avoid swallowing all exceptions in the wait loop.A blanket catch makes startup failures opaque. Catch expected transient exceptions and let unexpected ones surface.
♻️ Proposed adjustment
- catch { } + catch (HttpRequestException) + { + return false; + } + catch (System.Text.Json.JsonException) + { + return false; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs` at line 163, The empty catch in the CosmosDbBuilder wait loop swallows all exceptions; update the catch to only handle expected transient exceptions (e.g., HttpRequestException, TimeoutException, TaskCanceledException, SocketException) by catching those specific types (using an exception variable), log the error/retry attempt, and continue the loop, but rethrow or let any other unexpected exceptions bubble up so startup failures are visible; locate the silent catch inside the CosmosDbBuilder wait/retry method (the loop that polls Cosmos DB readiness) and replace the blanket catch with type-specific catches and logging/rehrow behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs`:
- Around line 36-39: The test currently obtains the database for assertion by
enumerating databases via
cosmosClient.GetDatabaseQueryIterator<DatabaseProperties>().ReadNextAsync() and
calling First(), which is order-dependent; instead, use the deterministic API to
read the created database directly by calling cosmosClient.GetDatabase("fakedb")
and invoking ReadAsync/ReadDatabaseAsync (or the equivalent synchronous read) on
that Database/DatabaseProperties handle returned after
CreateDatabaseIfNotExistsAsync; update the assertion source to use the result of
CreateDatabaseIfNotExistsAsync or the direct read of
cosmosClient.GetDatabase("fakedb") rather than querying and calling First().
---
Nitpick comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Line 163: The empty catch in the CosmosDbBuilder wait loop swallows all
exceptions; update the catch to only handle expected transient exceptions (e.g.,
HttpRequestException, TimeoutException, TaskCanceledException, SocketException)
by catching those specific types (using an exception variable), log the
error/retry attempt, and continue the loop, but rethrow or let any other
unexpected exceptions bubble up so startup failures are visible; locate the
silent catch inside the CosmosDbBuilder wait/retry method (the loop that polls
Cosmos DB readiness) and replace the blanket catch with type-specific catches
and logging/rehrow behavior.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cssrc/Testcontainers.CosmosDb/CosmosDbContainer.cstests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs (1)
26-32: Consider extractingCosmosClientcreation to a helper.Both tests create
CosmosClientwith identical configuration. A shared helper method would reduce duplication.♻️ Optional refactor
private CosmosClient CreateCosmosClient() { return new CosmosClient( _cosmosDbContainer.GetConnectionString(), new CosmosClientOptions { ConnectionMode = ConnectionMode.Gateway, HttpClientFactory = () => _cosmosDbContainer.HttpClient }); }Then use
using var cosmosClient = CreateCosmosClient();in each test.Also applies to: 51-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs` around lines 26 - 32, Extract the duplicated CosmosClient construction into a helper method (e.g., CreateCosmosClient) that returns a new CosmosClient configured with _cosmosDbContainer.GetConnectionString() and a CosmosClientOptions { ConnectionMode = ConnectionMode.Gateway, HttpClientFactory = () => _cosmosDbContainer.HttpClient }; then replace the inline constructions in both tests with using var cosmosClient = CreateCosmosClient(); so both occurrences reuse the same setup and avoid duplication.src/Testcontainers.CosmosDb/CosmosDbBuilder.cs (1)
114-114: Minor style: Add space afterif.📝 Suggested fix
- if(_httpClient is null) + if (_httpClient is null)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs` at line 114, The conditional in CosmosDbBuilder uses "if(_httpClient is null)" without a space after "if"; update the conditional that checks the _httpClient field (in class CosmosDbBuilder) to follow style guidelines by adding a space so it reads "if (_httpClient is null)" wherever that check appears.src/Testcontainers.CosmosDb/CosmosDbContainer.cs (1)
41-46: Internal health-check properties have the same instance-per-access pattern.
HttpMessageHealthCheckHandlerandHttpClientHealthCheckalso allocate new instances on each property access. Consider caching these as fields, especially sinceWaitUntilinCosmosDbBuilder.csalready mitigates this by storing the client in_httpClientafter first access.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.CosmosDb/CosmosDbContainer.cs` around lines 41 - 46, The two properties HttpMessageHealthCheckHandler and HttpClientHealthCheck create new instances on every access; change them to cached fields (e.g., private readonly or lazy-initialized fields like _httpMessageHealthCheckHandler and _httpClientHealthCheck) and initialize them once (or lazily) so the same UriRewriter and HttpClient instances are reused across accesses; update any existing property getters (HttpMessageHealthCheckHandler, HttpClientHealthCheck) to return those cached fields and ensure lifecycle/Dispose expectations are handled consistently (noting CosmosDbBuilder.WaitUntil already caches an _httpClient).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Around line 105-176: WaitUntil currently implements IDisposable and disposes
the HttpClient it obtains from CosmosDbContainer.HttpClientHealthCheck, but
Dispose() is never called causing a leak; remove IDisposable and the Dispose()
method, stop disposing the HttpClient, and treat the HttpClient as a
non-owning/shared instance (either keep the existing _httpClient as a simple
reference set from ((CosmosDbContainer)container).HttpClientHealthCheck inside
UntilAsync or replace with a static/shared HttpClient instance); update
references to RequestUri, _httpClient, UntilAsync and ensure no code calls
Dispose() on HttpClient from WaitUntil.
- Line 8: The default image constant CosmosDbImage currently points to the
preview tag
"mcr.microsoft.com/cosmosdb/linux/azure-cosmos-emulator:vnext-preview"; change
this so the preview image is not the default by either (a) replacing
CosmosDbImage with a stable/GA image tag (e.g., a specific GA tag or "latest")
or (b) making the preview opt-in: keep a stable default constant (e.g.,
CosmosDbStableImage), and add a builder option/constructor parameter or
environment-flag (e.g., bool usePreviewImage or UsePreviewImage()) to the
CosmosDbBuilder that, when true, returns the preview tag (CosmosDbPreviewImage)
instead of the stable default; update any usages of CosmosDbImage to use the
chosen property or method.
---
Nitpick comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Line 114: The conditional in CosmosDbBuilder uses "if(_httpClient is null)"
without a space after "if"; update the conditional that checks the _httpClient
field (in class CosmosDbBuilder) to follow style guidelines by adding a space so
it reads "if (_httpClient is null)" wherever that check appears.
In `@src/Testcontainers.CosmosDb/CosmosDbContainer.cs`:
- Around line 41-46: The two properties HttpMessageHealthCheckHandler and
HttpClientHealthCheck create new instances on every access; change them to
cached fields (e.g., private readonly or lazy-initialized fields like
_httpMessageHealthCheckHandler and _httpClientHealthCheck) and initialize them
once (or lazily) so the same UriRewriter and HttpClient instances are reused
across accesses; update any existing property getters
(HttpMessageHealthCheckHandler, HttpClientHealthCheck) to return those cached
fields and ensure lifecycle/Dispose expectations are handled consistently
(noting CosmosDbBuilder.WaitUntil already caches an _httpClient).
In `@tests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs`:
- Around line 26-32: Extract the duplicated CosmosClient construction into a
helper method (e.g., CreateCosmosClient) that returns a new CosmosClient
configured with _cosmosDbContainer.GetConnectionString() and a
CosmosClientOptions { ConnectionMode = ConnectionMode.Gateway, HttpClientFactory
= () => _cosmosDbContainer.HttpClient }; then replace the inline constructions
in both tests with using var cosmosClient = CreateCosmosClient(); so both
occurrences reuse the same setup and avoid duplication.
ℹ️ Review info
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cssrc/Testcontainers.CosmosDb/CosmosDbContainer.cstests/Testcontainers.CosmosDb.Tests/CosmosDbContainerTest.cs
Currently, the Previous versions are no longer supported, according to Microsoft documentation. emulator-release-notes Another important point is that these newer versions already support more efficient health check mechanisms. I have updated the PR to support these new mechanisms. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Testcontainers.CosmosDb/CosmosDbBuilder.cs (1)
162-162: Consider logging exceptions for debugging purposes.The empty
catchblock is acceptable for wait strategies (returningfalseon failure is correct), but swallowing exceptions silently can make debugging difficult when the emulator fails to start.💡 Optional: Add trace logging
- catch { } + catch (Exception ex) + { + // Log at trace level for debugging without affecting normal operation + System.Diagnostics.Debug.WriteLine($"CosmosDB health check failed: {ex.Message}"); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs` at line 162, The empty catch in CosmosDbBuilder.cs swallows exceptions and makes debugging hard; update the catch (around the wait strategy that currently returns false) to log the caught Exception (include ex.Message and stack trace) instead of doing nothing—use the existing logging mechanism if available (e.g., an ILogger instance or Trace/Debug) and keep the current behavior of returning false after logging so the wait strategy semantics remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/Testcontainers.CosmosDb/CosmosDbBuilder.cs`:
- Line 162: The empty catch in CosmosDbBuilder.cs swallows exceptions and makes
debugging hard; update the catch (around the wait strategy that currently
returns false) to log the caught Exception (include ex.Message and stack trace)
instead of doing nothing—use the existing logging mechanism if available (e.g.,
an ILogger instance or Trace/Debug) and keep the current behavior of returning
false after logging so the wait strategy semantics remain unchanged.
|
Unfortunately, Microsoft has not provided clear communication regarding their roadmap and plans. This PR has been open for a while, and the old I can merge |
I agree that we should proceed with the vnext-preview version. Even though this image is still in preview, it's more useful than the old (latest) one. The current latest (old) version has several issues, especially with warmup. Because of that, the risk of breaking changes is probably low, since the warmup time in the current latest version makes it almost unusable. |
HofmeisterAn
left a comment
HofmeisterAn
left a comment
There was a problem hiding this comment.
I double-checked the implementation in the other Testcontainers language implemenations and aligned the .NET implementation with them. They’re already using vnext-preview.
HofmeisterAn
changed the title
What does this PR do?
This PR updates the CosmosDb image to the new version Microsoft is working on
Linux-based emulator (preview)
GitHub Repository
Why is it important?
This new image significantly improves the container startup process, eliminates certificate-related workarounds, and integrates seamlessly with CI/CD pipelines like GitHub Actions. For reference, you can check some of my tests here and GitActions here.
Related issues
#1306
Details
Explorer UI Disabled
The explorer UI was disabled since the container's purpose is to test integration with the Cosmos DB API.
Internal Routing Configuration
For internal routing to work as expected, the API port and mapping port need to be configured with the same value. References.
CosmosClient Configuration
The CosmosClient must be configured to use Gateway mode. See the official guidance.
Summary by CodeRabbit