Skip to content

Conversation

@jmanucornejo
Copy link

I apologize in advance if I'm doing something wrong, this is one of my first times contributing to open source. All comments are appreciated and let me know if I can improve or have to do more for this to be considered.

Even though MySQL is not a standard Loco.rs database, there are users that run Loco with a MySQL Backend. It can be easily done adding some feature flags to seaorm.

But even with those flags, when running Loco.rs tests (for example in tests/models) and trying to seed fixtures to MySQL with "seed::(&boot.app_context).await.unwrap();", users will get this error returned "Unsupported database backend: MySQL".

I understand now it was because the tests go through specific functions in the db.rs which defaulted to errors on the MySQL backend arm, making it impossible to test using MySQL backend.

I have implemented the MySQL arms for each of the functions that where causing errors. I also created some tests for MySQL backend for the functions which can be run with:

cargo test --features testing test_mysql

My 3 tests passed. I tried to replicate the postgres way of testing.

I also tried the formatting in the CONTRIBUTING.MD but it changed things I didn't contribute to so I tried my best to format it as I saw in other files.

@kaplanelad
Copy link
Contributor

Hey @jmanucornejo, thanks for opening this PR!
Did you review this issue: #1294 ?

@jmanucornejo
Copy link
Author

I did see that issue when starting my project. In fact, when I mentioned in my PR the feature flags, I meant the sqlx-mysql on sea-orm mentioned on that same issue. Everything seemed to work fine with that feature flag, except tests.

Thats why I ended up doing this. I want to have MySQL for my tests in order for them to be consistent with my database backend. Apparently, the feature flag wasn't enough for tests. Seeding function kept throwing the "Unsupported database backend: MySQL".

@kaplanelad
Copy link
Contributor

I went over this and tested the scenario described in this comment:
#1294 (comment)

The issue appears to be related to the generator field. SeaORM generates different types for PostgreSQL and MySQL, which causes the mismatch.

I recommend running cargo loco generate model ... against a MySQL database to reproduce the issue and verify whether any errors occur.

@jmanucornejo
Copy link
Author

I'm not sure what that has to do with my pull request.

To clarify, I have no problem generating models and entities against my MySQL Backend.

The problem is specifically with running tests, and line 358, 400 and 452 in src/db.rs:

DatabaseBackend::MySql => { return Err(Error::Message( "Unsupported database backend: MySQL".to_string(), )); }

The MySQL arm directly returns an error, even with MySQL working correctly.

Can you please give me more detail as to what you want me to check about that comment?

@kaplanelad
Copy link
Contributor

Try generating a scaffold using the cargo loco generate scaffold command and add a variety of different db field types.
Then verify that the generated code compiles successfully.

@jmanucornejo
Copy link
Author

As I mentioned before, I have no problem creating models, entities, etc using cargo loco commands. It does compile.

The problem is with tests.

Please I really don't see how what you are saying is related to the PR.

@kaplanelad
Copy link
Contributor

Ok, if everything works on your end, the next step is to implement all the MySQL cases in the project and add MySQL integration tests here:
https://github.com/loco-rs/loco/blob/master/loco-new/tests/wizard/new.rs

@jmanucornejo
Copy link
Author

Can you specify how that is related to this PR? I still don't understand why you keep mentioning the scaffolding or the issue #1294 . My PR is about running tests, not creating a new app.

If you need help implementing the app creatiion with MySQL, I can check it out, but this is not what this PR is about.

@kaplanelad
Copy link
Contributor

We do not officially support SQL right now. Adding official support would require integrating it into our tests and validating all related functionality.

@jmanucornejo
Copy link
Author

So in summary, if I want to get this to work, I'd need to create a PR with all mysql flows fixed, not just the tests part? Its an all or nothing kind of thing? Let me know so I can see what it would take. Thanks for your time!

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.

2 participants