Xabaril / Esquio

Esquio is a Feature Toggle Library for .NET Developers.
Apache License 2.0
428 stars 49 forks source link

Multiple data stores #139

Closed Kronos11 closed 4 years ago

Kronos11 commented 4 years ago

Adds in functionality for #137

What this PR does / why we need it:

This PR adds Npgsql as a separate database provider by creating a separate project for NpgSql and SqlServer

Kronos11 commented 4 years ago

A couple of questions: @unaizorrilla

  1. Do we want SqlServer to be default and assume that by not requiring a UseSqlServer : true config?
  2. Do we want to setup the docker file to include both postgres and sqlserver?
  3. Do you want a sample project that uses postgres instead of sql server?
Kronos11 commented 4 years ago

I'm not quite sure what's going on, the Functional tests are working for both on my machine with docker running, should be nearly identical. Looks to be something with Respawn trying to reset the db

unaizorrilla commented 4 years ago

Checking the error there is some strange, postgress functional tests are using SqlClient? The origin is on Respawn Reset, probably respawn is not using postgress connection!

I try to review this on weekend! I have long work weeks this month :-(

Kronos11 commented 4 years ago

Fixed functional tests, I needed to set both env variables correctly

unaizorrilla commented 4 years ago

Can I review the PR?

Kronos11 commented 4 years ago

Please do

On Fri, Jul 3, 2020 at 12:42 AM Unai Zorrilla notifications@github.com wrote:

Can I review the PR?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/Xabaril/Esquio/pull/139#issuecomment-653402805, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACF4QUNE2WZ6JWWEFRXHO3RZWDUZANCNFSM4OLKS7AA .

unaizorrilla commented 4 years ago

Hi @Kronos11

I'm reviewing this! look good, I'm changing some things and when pushed we can discuss!

For example, the first thing i'm planning to do is modify ServerFixture to simplify adding more DbProviders in future ( UseNpgsql restrict only to npgsql or sqlserver )

thx again!

Kronos11 commented 4 years ago

Hi @Kronos11

I'm reviewing this! look good, I'm changing some things and when pushed we can discuss!

For example, the first thing i'm planning to do is modify ServerFixture to simplify adding more DbProviders in future ( UseNpgsql restrict only to npgsql or sqlserver )

thx again!

Totally, I was actually planning on making that change depending on what you were thinking with the config. We could also have it Store:Provider and then the options could be SqlServer, NpgSql, etc which can then be const lookups or something similar. I can work on it tomorrow if you’re good with it?

unaizorrilla commented 4 years ago

Let me some days! I’m doing some Changes and pushing my commits

unaizorrilla commented 4 years ago

Hi @Kronos11

I create a branch from this PR with my changes

https://github.com/Xabaril/Esquio/tree/Kronos11-feature/otherDbStores

Can you continue the PR on this branch? Close this and add your feedback on new branch?

My changes are :

Can you review this and continue the PR ( on this branch ) ? UI sample is not tested and probably need some customization on docker-compose ui to select the store to use etc

Thanks for all your work!!

Kronos11 commented 4 years ago

closing and continuing on #141