Pg Check #6
Pg Check #6
Conversation
src/holdup/cli.py
Outdated
| @@ -237,6 +270,12 @@ def parse_value(value, proto): | |||
| raise argparse.ArgumentTypeError('Invalid service spec %r. Port must be a number.' % value) | |||
| port = int(port) | |||
| return TcpCheck(host, port) | |||
| elif proto == 'postgresql': | |||
There was a problem hiding this comment.
For convenience a "pg" protocol should also be allowed.
There was a problem hiding this comment.
Also "postgres" (cause they allow it), and we normalize to "postgresql" internally.
src/holdup/cli.py
Outdated
| @@ -237,6 +270,12 @@ def parse_value(value, proto): | |||
| raise argparse.ArgumentTypeError('Invalid service spec %r. Port must be a number.' % value) | |||
| port = int(port) | |||
| return TcpCheck(host, port) | |||
| elif proto == 'postgresql': | |||
| user, password, host, port, dbname = value.strip('/').split(':', 4) | |||
There was a problem hiding this comment.
I think we should have a different syntax. For tcp protocol it's very clear and simple to parse (host:port) but for postgres we have to many things. postgres://user:password:host:port:dbname is not really intuitive.
Fortunately psycopg2 supports DSNs (https://www.postgresql.org/docs/current/libpq-connect.html#LIBPQ-CONNSTRING) so we can just use those and not do any parsing at all.
Parse with psycopg2.extensions.make_dsn and just pass its result to psycopg2.connect.
src/holdup/cli.py
Outdated
| def is_fatal_error(self, error): | ||
| representation = repr(error) | ||
| if 'password authentication failed' in representation or\ | ||
| 'database "{0}" does not exist'.format(self.database) in representation: |
There was a problem hiding this comment.
Considering that db not existing is analogous to dns failing to resolve a hostname, and auth failures is analogous to permission denied errors when checking file/uds (the file/unix checks) then it would be inconsistent to have this distinction on failures.
It also makes code more complicated, lets revert it.
revert additional error checks This reverts commit 3eb65ef.
Codecov Report
@@ Coverage Diff @@
## master #6 +/- ##
==========================================
- Coverage 89.68% 86.93% -2.76%
==========================================
Files 4 4
Lines 349 375 +26
Branches 60 62 +2
==========================================
+ Hits 313 326 +13
- Misses 22 34 +12
- Partials 14 15 +1
Continue to review full report at Codecov.
|
src/holdup/cli.py
Outdated
| uri = 'postgresql://{}'.format(value) | ||
| try: | ||
| connection_uri = make_dsn(uri) | ||
| except ProgrammingError: |
There was a problem hiding this comment.
Should capture all exceptions (except Exception as exc:) and raise argparse.ArgumentTypeError("Failed to parse %s: %s. Expected format is blabla" % (value, exc)) or similar.
Created a separate check, postgres specific to solve a bug where the postgres server would start but the database itself woudln't.