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

Add missing wait strategy to test if database containers are up and running (ODBC) #119

Closed
6 tasks done
HofmeisterAn opened this issue Aug 10, 2019 · 11 comments
Closed
6 tasks done
Labels
enhancement New feature or request good first issue Want to contribute to Testcontainers? Start from here.
Milestone

Comments

@HofmeisterAn
Copy link
Collaborator

HofmeisterAn commented Aug 10, 2019

Replace or extend WaitUntilPortsAreAvailable: https://github.com/HofmeisterAn/dotnet-testcontainers/blob/e23d1eb8b6ea20fbf748fa0158e67a88719415c1/src/DotNet.Testcontainers/Core/Wait/WaitUntilPortsAreAvailable.cs#L15

  • PostgreSql
  • MySql
  • MsSql
  • CouchDb
  • Redis
  • RabbitMq
@HofmeisterAn HofmeisterAn added enhancement New feature or request good first issue Want to contribute to Testcontainers? Start from here. labels Aug 10, 2019
@HofmeisterAn HofmeisterAn added this to the 1.0.0 milestone Aug 10, 2019
@HofmeisterAn HofmeisterAn changed the title Use ODBC connection to test if the database container is up and running Add missing wait strategy to test if database containers are up and running (ODBC) Aug 17, 2019
@lukasvavrek
Copy link
Contributor

Hello @HofmeisterAn, I would love to take a look at it.

Do you have something specific in mind in order to extend/replace this class? Can you point me to the right direction?

@HofmeisterAn
Copy link
Collaborator Author

I think the feature splits into two parts:

One in an additional implementation of IWaitUntil. In the implementation I would try to establish a database connection. If the connection is successful, the Testcontainer is ready for use. If the connection fails, I would try n-more times before finally throwing an exception. Unfortunately, it is not enough to test the database port availability.

The second part seems to be a bit more complicated. I would like to offer an implementation that supports as many databases as possible without additional providers. So far, I only tested a very simple implementations and had problems with Unix systems again and again. Maybe it is even necessary to fall back on the implementations of the providers and write a generic implementation with System.Data.Common. In this case I would probably prefer database specific modules. See also: .NET Framework Data Providers.

This feature does not require any special database functions. We just need to make sure that a connection is possible. Do you have any ideas for implementation?

@lukasvavrek
Copy link
Contributor

Thanks for providing all the information.
I believe that the implementation of the first part is clear.

When it comes to actual connectivity testing, I would say that we could have one implementation that will work on top of the DbConnection. For providers that are not compatible with this class, we might create Db-specific implementations or let the user provide it.

What do you think?

@HofmeisterAn
Copy link
Collaborator Author

Sounds great. I think we should provide specific implementations as own modules in the future. For now, we can focus on compatible implementations.

@HofmeisterAn
Copy link
Collaborator Author

@Ta-Ja Maybe we can use a much simpler implementation. Each database container should contain everything to connect to himself. We can use this mechanism and provide provider specific scripts, e. g. for PostgreSQL we can use pg_isready. This should be very simple, and we don't need any .NET database provider at all.

@lukasvavrek
Copy link
Contributor

@HofmeisterAn that looks great! I like the idea that we don't need to use a .NET DB provider. I'll take a look at it today.

@HofmeisterAn
Copy link
Collaborator Author

I've added a little test implementation for PostgreSql, looks good so far. I didn't have much time to clean up the code: https://github.com/HofmeisterAn/dotnet-testcontainers/compare/feature/implement-database-is-running.

@lukasvavrek
Copy link
Contributor

@HofmeisterAn I see that you are quite far with your implementation. I tried some implementation locally based on your suggestions but it's not comparable to your work.
If you still need some help with this task, please let me know.

@HofmeisterAn
Copy link
Collaborator Author

@Ta-Ja I have made some changes in preparation for version 1.0.0 too. In most of the changes, I refactored inconsistent namespaces. Unfortunately, some changes are breaking changes.

Anyway, I'm pretty interested in your ideas and thoughts. I like to share as much as I can. Maybe you can provide some of your details.

Are you working on your four pull request for hacktoberfest? If you like we can set up a pr for your efforts.

HofmeisterAn added a commit that referenced this issue Oct 12, 2019
…ServiceContainer'

{Use native Docker container commands to wait until database and message broker containers are ready to establish connections.}
@lukasvavrek
Copy link
Contributor

@HofmeisterAn yes. I decided to take advantage of hacktoberfest and do my first OSS contribution.
I prefer not to create a throwaway PR so I'll sync with your new changes and work on top of them.

Correct me if I'm wrong, but the original idea of a wait, that'll repeat something N times and throws afterward isn't still implemented. Would it be beneficial to have it even when not used as planned for waiting for DB? If yes, I could work a bit more on it and create a PR.

What do you think?

HofmeisterAn added a commit that referenced this issue Oct 13, 2019
…ServiceContainer'

{Use native Docker container commands to wait until database and message broker containers are ready to establish connections.}
@HofmeisterAn
Copy link
Collaborator Author

Correct me if I'm wrong, but the original idea of a wait, that'll repeat something N times and throws afterward isn't still implemented.

Yes, thats right. I took the existing WaitStrategy to implement this features. This class waits until the command is successful or throws an exception after a timeout.

Would it be beneficial to have it even when not used as planned for waiting for DB?

I'm sure it will.

HofmeisterAn added a commit that referenced this issue Oct 14, 2019
…ServiceContainer'

{Use native Docker container commands to wait until database and message broker containers are ready to establish connections.}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Want to contribute to Testcontainers? Start from here.
Projects
None yet
Development

No branches or pull requests

2 participants