Database#exists? is a false boolean

[katafrakt]

Method checking for database existence is modeled as one returning a boolean. However, the reality is that at least three states might bi returned: exists, does not exists, we don't know.

As shown in #228 this might be problematic. Let's looks at PostgreSQL-related code:

def exists?
  result = system_call.call("psql -t -A -c '\\list #{escaped_name}'", env: cli_env_vars)
  result.successful? && result.out.include?("#{name}|") # start_with?
end

If the result is not successful, for example due to permissions issue, we return false here. But the truth is that we don't know if it exists or not. The check failed, but the return value suggests that we checked and the database does not exist.

I see 3 possible ways to resolve this:

1. Keep the function as boolean, raise exception if the check failed – this is probably easiest to implement, but as with exceptions, it needs to be caught somewhere and generally breaks the execution flow
2. Rename to check_existence, return a Result monad. Either: Success(:exists), Success(:not_exists), Failure(:check_failed) – this would require adding dry-monads as a dependency of this gem, which is probably not such a big deal as Hanami in general requires dry-monads
3. Simplification of the above with returning an enum: :exists, :does_not_exist, :check_failed

[kyleplump]

for what its worth: i like the exception idea

i think in this case a break of the execution flow is warranted. if there was some issue like a permissions issue, i feel as though the expectation is that execution of the action or command would stop, and that error is communicated. so exists? either returns a boolean, or throws some new Hanami::CLI::Error

granted, the same thing could be implemented / combined with a monad response (exec_drop_command throws instead of exists?)

either way excited to see where you take it 😄

[katafrakt]

Yes, I think raising exception is justified here. So if there are no other opinions, I think I'd make a PR with that change.