search

memsql / dbt-singlestore

Apache License 2.0
7 stars 8 forks source link

improve UX of profile declaration and validation #1

Closed dataders closed 2 years ago

dataders commented 2 years ago

@amychen1776 and I were eventually able to connect but experienced the following pain points, that are fortunately fixable. In addition to the feedback below, I think it'd be a great idea to have some singlestore users go through the paces of connecting dbt to singlestore.

default vs optional credential values

if the default port is 3306, we should probably say in the docs that it's optional. especially if 99% of the time the port is 3306. Also does it make sense to have root be the default username? Why is the default database Optional but also None? shouldn't it be required?

profile keys synonyms

https://github.com/dbt-labs/docs.getdbt.com/pull/1044 says the profile should have user but the adapter only acceptsusername right now. while the docs should be accurate, it's also an easily remediable situation with the ALIASES dictionary.

friendlier error messages

in the above example where a user used user instead of username, the below error message was returned. Here's some examples of how dbt-firebolt does more user-friendly error-handling.


>Runtime Error
Database Error
  (1045, "Access denied for user 'root'@'100.97.252.127' (using password: YES)")

https://github.com/memsql/dbt-singlestore/blob/ba744c898bb7744b88c380d2d883970f543bc4e5/dbt/adapters/singlestore/connections.py#L19-L39

pmishchenko-ua commented 2 years ago
  1. I've modified SingleStoreCredentials to indicate which are optional and which are required, and I'm updating the docs accordingly.
  2. user is now used everywhere, not username. username is added as an alias for user
  3. I've updated error message a bit, hope it will be helpful.