Closed fblackburn1 closed 3 months ago
Hey, first of all thanks for the compliment and this PR. I was on vacation, therefore I couldn't take a look into the project. I will review this one asap.
Looks good to me :+1:
One thing that came to my mind: even with a environment variable PGPASSWORD
you still have to provide the password for the actual connection used for the anonymization, e.g.:
$ set PGPASSWORD password
$ pganonymize --schema schema.yml \
--dbname anonymization_test \
--user pganonymize \
--password password \
--dump-file test.dump
Maybe it would be a nice addition to re-use environment variables also for the default connection arguments from argparse
, e.g.:
$ set PGPASSWORD password
$ set PGUSER pganonymize
$ set PGDBNAME anonymization_test
$ pganonymize --schema schema.yml --dump-file test.dump
But that would be something for a separate pull request due to the changes for CLI parsing.
Looks good to me 👍
One thing that came to my mind: even with a environment variable
PGPASSWORD
you still have to provide the password for the actual connection used for the anonymization, e.g.:$ set PGPASSWORD password $ pganonymize --schema schema.yml \ --dbname anonymization_test \ --user pganonymize \ --password password \ --dump-file test.dump
From small tests, PGPASSWORD was not working before this PR (If I understand correctly, when omitted --password
, the code will send empty password without taking into consideration environment variable)
From my point of view, this PR only use PGPASSWORD internally, and does not intend to use PGPASSWORD from outside of the script, which is a separate feature
If I miss something or it breaks any workflow, let me know. I'll fix it :)
First of all, it's a very nice tool, good job :star_struck:
This PR allow to use password provided in the cli option for the
pg_dump
command Otherwise using--dump-file
option will ask for a password even if the password is given in the command