Core: Fix transfer command to use DSN connection parameters #2286
Conversation
The transfer command was using `sudo -u postgres psql` which connects to the system postgres user's default instance instead of respecting the onegov.yml DSN configuration. This allows to use ports other than 5432 locally. TYPE: Bugfix
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Daverball
left a comment
Daverball
left a comment
There was a problem hiding this comment.
Looks good overall, although there is a little bit of unnecessary parsing code duplication. Also there's a more robust way of setting an environment variable in a subprocess call.
Daverball
left a comment
Daverball
left a comment
There was a problem hiding this comment.
I don't think you need to care about the multi host part of MultiHostUrl, if you use the scalar properties they will return the first host. I also don't think we need to guard against empty hosts.
| hosts = local_dsn.hosts() | ||
| if hosts: | ||
| host_info = hosts[0] | ||
| local_host = host_info.get('host') or 'localhost' | ||
| local_port = str(host_info.get('port') or 5432) | ||
| local_user = host_info.get('username') or 'postgres' | ||
| local_password = host_info.get('password') or '' | ||
| else: | ||
| local_host = 'localhost' | ||
| local_port = '5432' | ||
| local_user = 'postgres' | ||
| local_password = '' # nosec: B105 |
There was a problem hiding this comment.
| hosts = local_dsn.hosts() | |
| if hosts: | |
| host_info = hosts[0] | |
| local_host = host_info.get('host') or 'localhost' | |
| local_port = str(host_info.get('port') or 5432) | |
| local_user = host_info.get('username') or 'postgres' | |
| local_password = host_info.get('password') or '' | |
| else: | |
| local_host = 'localhost' | |
| local_port = '5432' | |
| local_user = 'postgres' | |
| local_password = '' # nosec: B105 | |
| local_host = local_dsn.host or 'localhost' | |
| local_port = str(local_dsn.port or 5432) | |
| local_user = local_dsn.username or 'postgres' | |
| local_password = local_dsn.password or '' |
I believe this should work out to exactly the same result, hosts is only necessary if you want to handle more than one host.
| hosts = local_dsn._url.hosts() | ||
| if hosts: | ||
| host_info = hosts[0] | ||
| local_host = host_info.get('host') or 'localhost' | ||
| local_port = str(host_info.get('port') or 5432) | ||
| local_user = host_info.get('username') or 'postgres' | ||
| local_password = host_info.get('password') or '' | ||
| else: | ||
| local_host = 'localhost' | ||
| local_port = '5432' | ||
| local_user = 'postgres' | ||
| local_password = '' # nosec: B105 |
There was a problem hiding this comment.
| hosts = local_dsn._url.hosts() | |
| if hosts: | |
| host_info = hosts[0] | |
| local_host = host_info.get('host') or 'localhost' | |
| local_port = str(host_info.get('port') or 5432) | |
| local_user = host_info.get('username') or 'postgres' | |
| local_password = host_info.get('password') or '' | |
| else: | |
| local_host = 'localhost' | |
| local_port = '5432' | |
| local_user = 'postgres' | |
| local_password = '' # nosec: B105 | |
| local_host = local_dsn.host or 'localhost' | |
| local_port = str(local_dsn.port or 5432) | |
| local_user = local_dsn.username or 'postgres' | |
| local_password = local_dsn.password or '' |
Same thing here
Commit message
Core: Fix transfer command to use DSN connection parameters
The transfer command was using
sudo -u postgres psqlwhich connectsto the system postgres user's default instance instead of respecting
the onegov.yml DSN configuration. This allows to use ports other than
5432 locally. (Which is something you probably want to do to differentiate
between different PostgreSQL versions, if there are more than one installed.)
TYPE: Feature
Checklist