pass PGPASSWORD via env directly, not via shell

[evgeni]

No description provided.

[evgeni]

with this, nothing uses hidden_patterns anymore. should we drop it, so that this bad design doesn't creep up on us again?

and one more thing: technically hidden_patterns also replaces the pattern in stdout, but I would not expect Postgres to ever print the password, so 🤷‍♀️

[ehelms]

Agreed on cleaning up hidden_patterns if it's not used.

[ekohl]

Overall I think this is OK, but the whole module looks rather inconsistent.

[ekohl]

I think RuboCop would prefer

Suggested change

	return cmd, env
	cmd, env

[evgeni]

and yet it didn't complain :D
(I think it only complains on singular returns, not lists)

[ekohl]

It feels like this should use psql('SHOW server_version'), but you probably also saw that and considered it out of scope.

And as always, I think it should have used --no-align instead of -A to make the comment above it redundant.

[ekohl]

Should we just drop the file?

[evgeni]

we could, yeah. I pondered writing actual new tests, but didn't so far.

[evgeni]

I've just realized that while this fixes one issue with the passwords, it doesn't fix the one in the report. 🤦‍♀️
That is about us parsing pulp's settings.py wrongly

[ekohl]

I wonder if it could wrap django's dbshell command to run queries somehow

[evgeni]

Don't give me ideas

[evgeni]

PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=/etc/pulp python -c 'import yaml; import settings; print(yaml.safe_dump(settings.DATABASES["default"]))'

[ekohl]

My biggest issue is that the Rails equivalent needs to load the entire Rails env, which is slow. And there is none (that I know of) for Candlepin. Not that we need queries on that after we merge #940, but there's still the backup/restore part.

[ekohl]

PYTHONDONTWRITEBYTECODE=1 PYTHONPATH=/etc/pulp python -c 'import yaml; import settings; print(yaml.safe_dump(settings.DATABASES["default"]))'

Nit: I'd prefer JSON and you can use pulpcore-admin shell to get a Python shell with guaranteed the correct Python version.

[evgeni]

sure, why not:

# PULP_SETTINGS=/etc/pulp/settings.py pulpcore-manager shell --command 'from django.conf import settings; import json; print(json.dumps(settings.DATABASES["default"]))'
{"ENGINE": "django.db.backends.postgresql", "NAME": "pulpcore", "USER": "pulp", "PASSWORD": "sdfsfds\"dssdfdsf", "HOST": "remotedb", "PORT": "5432", "ATOMIC_REQUESTS": false, "AUTOCOMMIT": true, "CONN_MAX_AGE": 0, "CONN_HEALTH_CHECKS": false, "OPTIONS": {}, "TIME_ZONE": null, "TEST": {"CHARSET": null, "COLLATION": null, "MIGRATE": true, "MIRROR": null, "NAME": null}}

[ekohl]

The reason I bring it up is that it's probably easier to reuse this once we start using containers. If we start using env vars or secrets, that'll work at runtime but it'll be complex to parse all the options.

[evgeni]

#941