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

Make PgConnectOptions fields pub instead of pub(crate) #2764

Open
lcmgh opened this issue Sep 19, 2023 · 4 comments
Open

Make PgConnectOptions fields pub instead of pub(crate) #2764

lcmgh opened this issue Sep 19, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@lcmgh
Copy link

lcmgh commented Sep 19, 2023

When parsing db config from a custom struct that includes url: String one still may need access to the underlying parsed values such as host, port etc.

As you can see here the fields are all private: https://docs.rs/sqlx/latest/sqlx/struct.PgConnection.html

Use cases:

  • Health checks that get a ConnectionPool as config want to be aware of the actual host/port
  • AWS IAM based RDS authentication requires the databases hostname and port as input for generating temporary credentials
@lcmgh lcmgh added the enhancement New feature or request label Sep 19, 2023
@raunak-dave
Copy link

+1 for MySqlConnectOptions.

@andir
Copy link

andir commented Oct 25, 2023

Another use case for this is testing in application using e.g. rocket. Right now you've to hardcode the connect URL and manually modify it based on the database name you can get from PgConnectOptions. https://wtjungle.com/blog/integration-testing-rocket-sqlx/ is a good example of what currently has to be done to execute tests with sqlx support. It shouldn't be that hard.

Would a simple PR that makes these fields pub be accepted?

@abonander
Copy link
Collaborator

abonander commented Nov 4, 2023

Public fields are a semver hazard that I'm not ready to accept.

I just recently merged a PR to add a getter for the hostname to PgConnectOptions: #2752

I would accept PRs to add getters for the port, and to add both getters to MySqlConnectOptions.

@andir
Copy link

andir commented Nov 5, 2023

The way I solved this right now was parsing the DATABASE_URL and updating the database (path) in the URL to whatever sqlx is using:

    async fn get_test_client(pg_connect_options: PgConnectOptions) -> Client {
        use url::Url;
        lazy_static::lazy_static! {
            static ref ENV_DB_URL: Url = {
            let env_database_url = std::env::var("DATABASE_URL").unwrap();
            Url::parse(&env_database_url).unwrap()
            };
        }

        let db_name = pg_connect_options.get_database().unwrap();

        let db_url = {
            let mut u = ENV_DB_URL.clone();
            u.set_path(db_name);
            u
        };

        let db_url = db_url.to_string();

        Client::tracked(build_rocket(&db_url)).await.unwrap()
    }

It is a few lines of code but given that the database is usually define with that environment variable and since sqlx isn't changing port or host that should be good enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants