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: Read replicas connection settings replaced by master settings (continuation) #2081

Closed

Conversation

stevenhansel
Copy link

Based on the previous pull request for fixing #1963

This PR adds the unit test to check if MikroORM is initialized with replicas, when getting the connection with the params of read, it should return the configuration with the replicas db configuration.

@B4nan
Copy link
Member

B4nan commented Aug 2, 2021

Are the tests passing for you locally? Because I see you did the very same change as in the old PR but many tests were failing.

@stevenhansel
Copy link
Author

stevenhansel commented Aug 2, 2021

@B4nan Sometimes i do get an error like database_name of x is not found when running all the tests. So, I thought that those tests were failing because of my testing setup or something else. I also tried testing only on the unit tests that I made, and they worked perfectly fine.

Do you have any hints regarding why those errors are showing? because I also tried running the tests in the master branch, and all the tests passed.

@B4nan
Copy link
Member

B4nan commented Aug 2, 2021

Well, CI tests are not passing, and they were not passing with the previous PR too, so this will be most probably something wrong with the change you made in code. I don't have any hints, the tests should create databases automatically.

Or it might as well be some other part of code that just relied on the old (and buggy) behaviour.

@stevenhansel
Copy link
Author

Well, CI tests are not passing, and they were not passing with the previous PR too, so this will be most probably something wrong with the change you made in code. I don't have any hints, the tests should create databases automatically.

Or it might as well be some other part of code that just relied on the old (and buggy) behaviour.

I will investigate further on what's causing this and find a fix

@B4nan
Copy link
Member

B4nan commented Aug 2, 2021

Let me know if you struggle with that, I can take a look locally too. I'd expect it will be something in the DatabaseDriver.createReplicas method.

Or even something with the MySQL tests that are using read replicas.

@B4nan
Copy link
Member

B4nan commented Aug 26, 2021

fixed via #2152

@B4nan B4nan closed this Aug 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants