postgres_database.absent reports the wrong error if the database is in use

[AstraLuma]

Description of Issue/Question

postgres_database.absent reports Database brokermate is not present, so it cannot be removed when the database exists but is in use (and therefore cannot be removed).

Setup

Debian master, minion, witth postgresql 9.6.3 installed.

app-database:
  postgres_database.absent:
    - name: app

Steps to Reproduce Issue

1. Have the above state
2. Have application running and pointing to target database
3. Attempt to apply above state

Versions Report

Dependency Versions:
           cffi: 1.9.1
       cherrypy: unknown
       dateutil: 2.5.3
      docker-py: Not Installed
          gitdb: 2.0.2
      gitpython: 2.1.5
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: 1.3.7
      pycparser: 2.17
       pycrypto: 2.6.1
   pycryptodome: Not Installed
         pygit2: Not Installed
         Python: 2.7.13 (default, Jan 19 2017, 14:48:08)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.1
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1
 
System Versions:
           dist: debian 9.0 
        machine: x86_64
        release: 4.9.0-3-amd64
         system: Linux
        version: debian 9.0 

Salt Version:
           Salt: 2016.11.2
 
Dependency Versions:
           cffi: Not Installed
       cherrypy: Not Installed
       dateutil: 2.5.3
          gitdb: Not Installed
      gitpython: Not Installed
          ioflo: Not Installed
         Jinja2: 2.8
        libgit2: Not Installed
        libnacl: Not Installed
       M2Crypto: Not Installed
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: Not Installed
       pycrypto: 2.6.1
         pygit2: Not Installed
         Python: 2.7.13 (default, Jan 19 2017, 14:48:08)
   python-gnupg: Not Installed
         PyYAML: 3.12
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: Not Installed
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1
 
System Versions:
           dist: debian 9.1 
        machine: x86_64
        release: 4.9.0-3-amd64
         system: Linux
        version: debian 9.1 

[gtmanfred]

I am able to replicate this. Would you be able to submit a PR to fix this?

If not, I have tagged this as an bug.

Thanks,
Daniel

[eradman]

Fixing this erroneous error message seems to require extensive changes to modules/postgres.py and states/postgres_database.py because these functions return either true/false or a list of results. Hence the API cannot distinguish between failure modes such as

• The database server is not running
• The database server rejected your credentials
• The database is in use or permission was denied
• The database does not exist

And so on. One might imaging changing the comment to be less presumptuous about why the operation failed, but without more complete error information this would create strange results for test=True

--- a/salt/states/postgres_database.py
+++ b/salt/states/postgres_database.py
@@ -216,6 +216,8 @@ def absent(name,
             return ret

     # fallback
-    ret['comment'] = 'Database {0} is not present, so it cannot ' \
-                     'be removed'.format(name)
+    if __opts__['test']:
+        ret['comment'] = 'Unable to remove the database \'{0}\''.format(name)
+    else:
+        ret['comment'] = 'Failed to remove the database \'{0}\''.format(name)
     return ret

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[AstraLuma]

TBH, I would be happy with a more vague error message. Like "Unable to remove database. Possible reasons include there was a connection issue, the database doesn't exist, or the database is in use."

Of course, bubbling up the error message would be the much better UX.

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[guettli]

Please pass through the original error message from db_remove(). Please do not invent a new vague error message.

This error lead to wasting a lot of time.