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

Support MySQL #18

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Support MySQL #18

wants to merge 13 commits into from

Conversation

waste-of-kindergarten
Copy link
Collaborator

resolve #17

@ShreckYe ShreckYe deleted the branch main November 29, 2024 15:04
@ShreckYe ShreckYe closed this Nov 29, 2024
@ShreckYe ShreckYe reopened this Nov 29, 2024
@ShreckYe ShreckYe changed the base branch from dev-dependent-on-snapshots to main November 29, 2024 15:08
@ShreckYe
Copy link
Member

ShreckYe commented Nov 29, 2024

I merged dev-dependent-on-snapshots into main and deleted the dev-dependent-on-snapshots branch, and it seems this PR was automatically closed. I pushed the dev-dependent-on-snapshots branch back so I could reopen this PR, and changed the merge target to main.

@ShreckYe
Copy link
Member

ShreckYe commented Nov 29, 2024

Please merge from main to get the latest changes. After that, you won't need to run publishToMavenLocal in the dependency projects anymore. And please see the changes in the "postgresql" module since this branch was created, and update the "mysql" module correspondingly. In IntelliJ IDEA, you can right click on the "postgresql" directory and use "Git -> Compare with Branch..." to do this.

Copy link
Member

@ShreckYe ShreckYe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use "Mysql" in our own code.

@waste-of-kindergarten waste-of-kindergarten marked this pull request as ready for review January 18, 2025 08:51
@ShreckYe ShreckYe self-assigned this Jan 18, 2025
Copy link

Copy link
Member

@ShreckYe ShreckYe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some code is not completely adapted. Try searching the words "pg" and "postgre" in the "mysql" directory to make sure no PostgreSQL code is not adapted.

import io.vertx.sqlclient.Pool
import io.vertx.sqlclient.SqlConnection

suspend fun <T> DatabaseClient<Pool>.withSQLTransaction(function: suspend (DatabaseClient<SqlConnection>) -> T): T =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one should be withMysqlTransaction and the connection type should be MySQLConnection.

)

@ExperimentalEvscApi
@JvmName("exposedDatabaseConnectMySQLWithParameterConnectionConfig")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@JvmName("exposedDatabaseConnectMySQLWithParameterConnectionConfig")
@JvmName("exposedDatabaseConnectMysqlWithParameterConnectionConfig")

*/
// made not inline anymore for easier debugging
@ExperimentalEvscApi
fun <SqlClientT : SqlClient, ClientBuilderT : ClientBuilder<SqlClientT>> createGenericPgClientWithBuilder(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Mysql" not "Pg".

MySQLPoolOptions(PoolOptions()) // remain to verify
)

fun createPgClient(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Mysql" not "Pg".

/**
* @see createGenericSqlClient
*/
fun createPgPool(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Mysql" not "Pg".

* @see createGenericSqlClient
*/
@Untested
suspend fun createPgConnection(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Mysql" not "Pg".

@ShreckYe
Copy link
Member

ShreckYe commented Jan 18, 2025

This PR is basically complete. We should just wait for #23 to be resolved to merge this.

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.

Support MySQL
2 participants