tconbeer / harlequin

The SQL IDE for Your Terminal.
https://harlequin.sh
MIT License
3.67k stars 84 forks source link

Bug: Conflicts between adapters using CLI options with same name #432

Closed alexmalins closed 8 months ago

alexmalins commented 8 months ago

Describe the bug It looks like CLI options of adapters are not firewalled between each other, so if two adapters have the same CLI option conflicts occur.

To Reproduce In a clean venv, pip install harlequin-mysql to install harlequin and mysql adapter. Running harlequin --help returns the correct --host description for MySQL:

mysql Adapter Options 
--host                                   -h       The host name or IP address of the MySQL server. (TEXT)

Next pip install harlequin-postgres and re-run harlequin --help:

mysql Adapter Options
--host                                   -h       Specifies the host name of the machine on which the server is running. 
                                                    If the value begins with a slash, it is used as the directory for the
                                                    Unix-domain socket.
                                                    (TEXT)

...

postgres Adapter Options                                                                                                                           
--host                                   -h      Specifies the host name of the machine on which the server is running.    
                                                   If the value begins with a slash, it is used as the directory for the     
                                                   Unix-domain socket.
                                                    (TEXT) 

Now the Postgres --host description has conflicted and overwritten the MySQL --host, probably as Postgres is lower alphabetically. Trying to connect to mysql with a --host CLI flag set results in a None type getting passed to the MySQL connector constructor (instead of the string you wrote in the command lint after --host).

Expected behavior Harlequin needs to completely separate and decouple the CLI options between adapters.

Actual behavior See above.

Additional context What is the output of harlequin --version? 1.13.0

What database adapter are you using with Harlequin? (Default is DuckDB) MySQL & Postgres

Can you tell us more about your system?

tconbeer commented 8 months ago

Thanks for the report -- I anticipated this; I'm not sure how to fix it. We may need to dig into the rich-click internals and see if we can override the help text for each specific grouping. Otherwise we could concatenate the help text.

I believe this only impacts the help text -- the options are passed through to the adapters correctly (if adapters implement validation, this could impact that as well).

alexmalins commented 8 months ago

I believe this only impacts the help text -- the options are passed through to the adapters correctly

It's more than this - when testing locally with Databricks & Postgres, I was finding the --username set wasn't getting passed through to the Databricks adapter. Potentially because under the hood a click option group is made for each adapter, so the --username flag for the Postgres adapter does not pass through the string to the Databricks adapter.

I didn't try with MySQL & Postgres as I don't have a MySQL to hand - could you please check if you can connect to a MySQL instance with a username if you've got the Postgres adapter installed as well?

tconbeer commented 8 months ago

The idea of option groups is just a rich-click thing, and it only impacts the formatting of the help screen. I can't reproduce what you're describing, but I do see a few ways that options could conflict with each other, and I think we can solve for most of them