theseus-rs / postgresql-embedded

Embed PostgreSQL database
Apache License 2.0
43 stars 5 forks source link

feat: adds a dump_schema function #43

Closed helio-frota closed 3 months ago

helio-frota commented 4 months ago

related to Err(error) => Err(DropDatabaseError(error.into())),

I have no idea if I should create another error enum variant, then I decided to send the PR this way to get the review first :+1:

bobmcwhirter commented 4 months ago

@helio-frota maybe prefer CommandError

https://github.com/theseus-rs/postgresql-embedded/blob/ff47c155bb7294afc51cebc4e42d5bab686cd80b/postgresql_embedded/src/error.rs#L14

helio-frota commented 4 months ago

@bobmcwhirter thanks :+1: PR updated

helio-frota commented 4 months ago

Instead of proliferating additional functions on the PostgreSQL API

Yeah, I thought twice about this -- before the PR.

+1 with approach of not proliferating things.

Again on this part:

https://github.com/theseus-rs/postgresql-embedded/blob/main/postgresql_embedded/src/command/mod.rs#L6-L9

If so, they will likely be moved to a separate public crate.

The usage seems good to me:

let settings: Settings = postgresql.settings();
let pg_dump = PgDumpBuilder::from(settings)
    .dbname(database_name)
    .schema_only()
    .file(output);

Feedback / PR reason:

We are running cargo test and SeaORM does some things on the database, it would be good for us to get the DB dump and transform that to a ER diagram (based on this dump file).

But now it seems to me that, instead of PR, it would be good to create an issue instead? (due to early stages, etc. (we don't have the pg_dump external crate yet))

@bobmcwhirter @brianheineman ideas ?

thanks

brianheineman commented 3 months ago

@helio-frota / @bobmcwhirter I refactored the commands out into a separate crate and updated them so that commands can be created from settings: https://github.com/theseus-rs/postgresql-embedded/pull/49

Please take a look and let me know if this will address your use case.

helio-frota commented 3 months ago

@brianheineman thanks for the updates :+1:

I was trying to use and I got

...Qpr3/bin/pg_dump" "--schema-only" "--host" "localhost" "--port" "0" "--username" "postgres"
Error: No such file or directory (os error 2)

for now I'm only trying to get the dump with the following code:

+let cloned_settings = settings.clone();
let mut postgresql = PostgreSQL::new(PostgreSQL::default_version(), settings);
postgresql.setup().await?;
postgresql.start().await?;

+log::debug!("{:?}", cloned_settings.binary_dir());
+let mut foobar = PgDumpBuilder::from(&cloned_settings)
+        .schema_only()
+        .program_dir(cloned_settings.binary_dir())
+        .build();
+let (stdout, stderr) = foobar.execute()?;
+log::debug!("{}", stdout);
+log::debug!("{}", stderr);

On the logs I can see:

2024-04-03T17:37:52.070388Z DEBUG start{self=PostgreSQL { version: Version { major: 16, minor: Some(2), release: Some(3) }, settings: Settings { installation_dir: "/tmp/.tmpzDQpr3/16.2.3", password_file: "/tmp/.tmpVgNdbp/.pgpass", data_dir: "/tmp/.tmpvKgIjG", host: "localhost", port: 0, username: "postgres", password: "trustify", temporary: true, timeout: Some(5s) } }}: postgresql_embedded::postgresql: Started database /tmp/.tmpvKgIjG on port 33355
[2024-04-03T17:37:52Z DEBUG trustify_common::db] "/tmp/.tmpzDQpr3/bin"

I don't know what is happening but I suspect I'm missing something with the installation_dir

Settings { installation_dir: "/tmp/.tmpzDQpr3/16.2.3"

because the debug log shows the bin without pg-version-dir [2024-04-03T17:37:52Z DEBUG trustify_common::db] "/tmp/.tmpzDQpr3/bin"

probably the correct should be /tmp/.tmpzDQpr3/16.2.3/bin [?] :shrug:

brianheineman commented 3 months ago

@helio-frota based on the output provided, it appears that the settings being used for the command have not been obtained from the postgresql reference. There are several arguments that should be set in the command such as the port that the server has started on; this still appears to be set to the default of 0 based on the output you provided. I created an integration test specifically for this use case that should illustrate how the commands can be used with postgresql_embedded. Please take a look at #50 and let me know if that works for you.

helio-frota commented 3 months ago

@brianheineman thanks for the response and that test :+1:

https://github.com/theseus-rs/postgresql-embedded/pull/50/files#diff-7ebd0daeeb8d86255fafd3733f9e0d6af0a1c7f2f5dd38fa51f796ff8d38e869R15

I changed the code to use let mut foobar = PgDumpBuilder::from(postgresql.settings()) and it worked. In fact, we are using this approach already: https://github.com/trustification/trustify/blob/main/common/src/db/mod.rs#L188

I think we can close this PR :+1: , thanks again