feat: add connection info to command on the CLI by Kayrnt · Pull Request #3356 · SQLMesh/sqlmesh

Kayrnt · 2024-11-10T21:03:18Z

Description

Adds connection details to help users to ensure that the project is configured as expected.
The output looks like the following:

sqlmesh -p ./sqlmesh_project info
Models: 1
Macros: 0


Connection:
  concurrent_tasks: 1
  execution_project: my-great-project
  job_retries: 1
  keyfile: '****'
  location: US
  method: oauth
  pre_ping: false
  priority: interactive
  project: my-great-project
  register_comments: true
  scopes:
  - https://www.googleapis.com/auth/bigquery
  type: bigquery




Test Connection:
  concurrent_tasks: 1
  execution_project: my-great-project
  job_retries: 1
  keyfile: '****'
  location: US
  method: oauth
  pre_ping: false
  priority: interactive
  project: my-great-project
  register_comments: true
  scopes:
  - https://www.googleapis.com/auth/bigquery
  type: bigquery




State Connection:
  concurrent_tasks: 1
  connector_config: {}
  database: local_state.db
  extensions: []
  pre_ping: false
  register_comments: true
  type: duckdb


Data warehouse connection succeeded
State backend connection succeeded

izeigerman · 2024-11-11T19:57:10Z

sqlmesh/core/context.py


+    def get_sensitive_fields(self) -> Set[str]:
+        """Returns a set of common sensitive field names"""
+        return {


There's already a similar list here: https://github.com/TobikoData/sqlmesh/blob/main/sqlmesh/core/config/scheduler.py#L110-L125. Can we unify them?

Great idea! Thanks for pointing out!

izeigerman · 2024-11-11T19:57:20Z

sqlmesh/core/context.py

                strict=strict,
            )

+    def get_sensitive_fields(self) -> Set[str]:


Shouldn't be a method

Should it be a just a value then?

izeigerman · 2024-11-11T19:57:40Z

sqlmesh/core/context.py

+            "ssh_key",
+        }
+
+    def is_sensitive_field(self, field_name: str, sensitive_fields: Set[str]) -> bool:


Shouldn't be a method. Let's move this to utils along with the list of sensitive fields.

izeigerman · 2024-11-11T19:57:45Z

sqlmesh/core/context.py

+        field_lower = field_name.lower()
+        return any(sensitive in field_lower for sensitive in sensitive_fields)
+
+    def mask_sensitive_value(self, value: Any) -> str:


izeigerman · 2024-11-11T19:58:42Z

sqlmesh/core/context.py

+            return "*" * len(str(value))
+        return "None"
+
+    def print_config(self, config: ConnectionConfig | None, console: Any) -> None:


There's already self available, why pass console as an argument? Also let's make this private (prefix with _). We should be careful about adding public methods to the Context since it's a gateway into SQLMesh Python API.

Could also be a candidate for utils

Ok, do you prefer that I create a dedicated utils file for that?

izeigerman · 2024-11-11T20:00:24Z

sqlmesh/core/context.py

        self.console.log_status_update(f"Models: {len(self.models)}")
        self.console.log_status_update(f"Macros: {len(self._macros) - len(macro.get_registry())}")

+        self.console.log_status_update("\n\nConnection:")


Should we make the headers more pronounced? Otherwise they blend in with individual connection attributes.

How would you do that?

You can make it bold by adding appropriate tags, eg:

self.console.log_status_update("\n\n[bold]Connection:[/bold]")

Ideally we also want to indent everything that follows under the configuration. This way we may not even need to do the highlighting.

Actually, can we just put this into dict and serialize as YAML instead? Eg.:

config = {"Test Connection": { ... }}

Ok I can try that

Kayrnt · 2024-11-11T22:20:01Z

@izeigerman, I moved the code but I didn't make any change yet regarding the template for the info output.

izeigerman · 2024-11-12T16:14:26Z

sqlmesh/utils/info.py

@@ -0,0 +1,71 @@
+from typing import Any, Optional, Set


Let's call make this module more descriptive by calling it config.py since everything here is related to configuration.

Alright! I updated the PR.

izeigerman · 2024-11-12T18:11:45Z

sqlmesh/utils/config.py

+        console.log_status_update("No connection configuration found.")
+        return
+
+    configDict = config.dict(mode="json")


Suggested change

configDict = config.dict(mode="json")

config_dict = config.dict(mode="json")

izeigerman · 2024-11-12T18:11:53Z

sqlmesh/utils/config.py

+
+    configDict = config.dict(mode="json")
+
+    for field_name in configDict:


Suggested change

for field_name in configDict:

for field_name in config_dict:

izeigerman · 2024-11-12T18:12:45Z

sqlmesh/utils/config.py

+from typing import Any, Optional, Set
+
+from sqlmesh.core.config.connection import ConnectionConfig
+import yaml


from sqlmesh.utils import yaml

setup.py

github-christophe-oudar · 2024-11-13T07:33:05Z

Thank you for the review and merge 🎉

feat: add connection info to command on the CLI

0899693

Kayrnt force-pushed the info-connection branch from 2ba8ff2 to 0899693 Compare November 10, 2024 21:25

add pydantic v1 support for print_config

83867e7

Kayrnt force-pushed the info-connection branch from 2370124 to 83867e7 Compare November 10, 2024 23:15

Kayrnt marked this pull request as ready for review November 10, 2024 23:26

izeigerman reviewed Nov 11, 2024

View reviewed changes

Kayrnt force-pushed the info-connection branch from 338144b to ac3f868 Compare November 11, 2024 21:52

review: move print config in dedicated utils

ae6cf4d

Kayrnt force-pushed the info-connection branch from ac3f868 to ae6cf4d Compare November 11, 2024 22:05

izeigerman reviewed Nov 12, 2024

View reviewed changes

Kayrnt added 2 commits November 12, 2024 17:19

review: rename info module to config

72a925a

review: improve formatting for config output

53fb066

Kayrnt force-pushed the info-connection branch from 1592ef7 to 53fb066 Compare November 12, 2024 17:28

izeigerman reviewed Nov 12, 2024

View reviewed changes

setup.py Outdated Show resolved Hide resolved

Kayrnt force-pushed the info-connection branch from 16199c0 to cabfb62 Compare November 12, 2024 18:34

review: small style changes

2482c87

Kayrnt force-pushed the info-connection branch from cabfb62 to 2482c87 Compare November 12, 2024 19:06

izeigerman approved these changes Nov 13, 2024

View reviewed changes

izeigerman merged commit c66efd3 into SQLMesh:main Nov 13, 2024

	configDict = config.dict(mode="json")
	config_dict = config.dict(mode="json")


		configDict = config.dict(mode="json")

		for field_name in configDict:

Conversation

Kayrnt commented Nov 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kayrnt commented Nov 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-christophe-oudar commented Nov 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Kayrnt commented Nov 10, 2024 •

edited

Loading