fix: return auth_type even if credentials are not in config

[aucampia]

Change dbsqlcli.main.apply_credentials_from_cfg to return 4 values even if not cfg.get("credentials") is true.

This is so that the CLI can be used only with environment variables.

• Fixes dbsqlcli.main.apply_credentials_from_cfg returns only three values when not cfg.get("credentials") #46

[susodapop]

Just confirming: this change has already been applied to the main branch as of four months ago. We can certainly add the test but it's not clear to me why anything would be "fixed" by applying this patch. What have I missed?

[aucampia]

Just confirming: this change has already been applied to the main branch as of four months ago.

I just rebased this onto your main branch, but the PR still shows a diff for dbsqlcli/main.py.

If it was applied, why is it still showing up in diff?

And why, if I revert the changes this PR makes to dbsqlcli/main.py, does the test fail?

20230511T003419W19 iwana@teekai.zoic.eu.org:~/sw/d/github.com/aucampia/databricks-sql-cli
$ git log -2
commit bbc776aa534f2b7e6b4ad1d9da6953ef727bf31e (HEAD -> aucampia-patch-1, origin/aucampia-patch-1)
Author: Iwan Aucamp <aucampia@gmail.com>
Date:   Tue Apr 18 22:21:07 2023 +0000

    add a test

commit 370a88295ee40513d4d634b32069b56407a3ce1e
Author: Iwan Aucamp <aucampia@gmail.com>
Date:   Tue Apr 18 01:56:22 2023 +0200

    fix: return auth_type even if credentials are not in config
    
    Change `dbsqlcli.main.apply_credentials_from_cfg` to return 4 values even if `not cfg.get("credentials")` is true.
    
    This is so that the CLI can be used only with environment variables.
    
    - Fixes <https://github.com/databricks/databricks-sql-cli/issues/46>
20230511T003423W19 iwana@teekai.zoic.eu.org:~/sw/d/github.com/aucampia/databricks-sql-cli
$ git diff
diff --git a/dbsqlcli/main.py b/dbsqlcli/main.py
index 41d5b4e..3672be7 100644
--- a/dbsqlcli/main.py
+++ b/dbsqlcli/main.py
@@ -64,7 +64,7 @@ def apply_credentials_from_cfg(hostname, http_path, access_token, auth_type, cfg
     """
 
     if not cfg.get("credentials"):
-        return hostname, http_path, access_token, auth_type
+        return hostname, http_path, access_token
 
     hostname = hostname or cfg.get("credentials", {}).get("host_name")
     http_path = http_path or cfg.get("credentials", {}).get("http_path")
20230511T003428W19 iwana@teekai.zoic.eu.org:~/sw/d/github.com/aucampia/databricks-sql-cli
$ poetry run pytest -- test/test_cli.py 
============================================================================ test session starts ============================================================================
platform linux -- Python 3.9.16, pytest-7.2.0, pluggy-1.0.0
rootdir: /home/iwana/sw/d/github.com/aucampia/databricks-sql-cli
collected 4 items                                                                                                                                                           

test/test_cli.py ...F                                                                                                                                                 [100%]

================================================================================= FAILURES ==================================================================================
____________________________________________________________________ test_passthrough_with_empty_config _____________________________________________________________________

    def test_passthrough_with_empty_config():
>       host_name, http_path, access_token, auth_type = apply_credentials_from_cfg(
            HOST_NAME, HTTP_PATH, ACCESS_TOKEN, AuthType.DATABRICKS_OAUTH.value, {}
        )
E       ValueError: not enough values to unpack (expected 4, got 3)

test/test_cli.py:65: ValueError
========================================================================== short test summary info ==========================================================================
FAILED test/test_cli.py::test_passthrough_with_empty_config - ValueError: not enough values to unpack (expected 4, got 3)
======================================================================== 1 failed, 3 passed in 0.24s ========================================================================

[aucampia]

Here is the diff I see in the "Files Changed" tab for this PR:

Can you create a permalink to where this is already applied?

[susodapop]

I apologise for the confusion on this. Looking through this with fresh eyes this morning I agree with your synopsis. This change LGTM.

[susodapop]

This has been released in https://github.com/databricks/databricks-sql-cli/releases/tag/v0.3.2