-
Notifications
You must be signed in to change notification settings - Fork 65
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added SQLDabatase connectivity #432
base: main
Are you sure you want to change the base?
Conversation
@microsoft-github-policy-service agree |
src/sempy_labs/_sqldatabase.py
Outdated
(workspace_name, workspace_id) = resolve_workspace_name_and_id(workspace) | ||
|
||
responses = _base_api( | ||
reqeust=f"/v1/workspaces/{workspace_id}/sqldatabases", uses_pagination=True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have you tested the code to make sure it's working? The parameter 'request' is spelled incorrectly.
The create and delete operation for SQLDatabase don't seem to be publicly available yet through API. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this PR. Just some changes to align to standards and for it to work. Please test this before making a PR in the future. Running this code would have failed in several places.
@@ -49,6 +49,11 @@ def __init__( | |||
(resource_id, resource_name) = resolve_item_name_and_id( | |||
item=item, type=endpoint_type.capitalize(), workspace=workspace_id | |||
) | |||
if endpoint_type == "sqldatabase": | |||
# SQLDatabase is has special case for resolving the name and id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be (resource_name, resource_id) = resolve_item_name_and_id(...)
class ConnectSQLDatabase(ConnectBase): | ||
def __init__( | ||
self, | ||
sqldatabase: str, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please make the parameter sql_database
timeout: Optional[int] = None, | ||
): | ||
super().__init__( | ||
name=sqldatabase, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be item=sql_database. not name=sqldatabase
# ) | ||
|
||
|
||
def list_sqldatabses(workspace: Optional[str | UUID] = None) -> pd.DataFrame: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's make this list_sql_databases to conform to the other function names. please prefix this function with an underscore as the API is not yet public so I do not want to release it yet. We can keep it as a private function for now.
for r in responses: | ||
for v in r.get("value", []): | ||
prop = v.get("properties", {}) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
names do not align with the column names. please align.
# ) | ||
|
||
|
||
def get_sqldatabase_tables( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_sql_database_tables
|
||
|
||
def get_sqldatabase_tables( | ||
sqldatabase: str | UUID, workspace: Optional[str | UUID] = None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameter: sql_database
|
||
Parameters | ||
---------- | ||
sqldatabase : str | uuid.UUID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
parameter: sql_database
return df | ||
|
||
|
||
def get_sqldatabase_columns( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_sql_database_columns
|
||
Parameters | ||
---------- | ||
sqldatabase : str | uuid.UUID |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sql_database
A pandas dataframe showing the SQLDabatases within a workspace. | ||
""" | ||
|
||
columns = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The columns should be:
"SQL Database Name"
"SQL Database Id"
"Description"
"Connection Type"
"Connection Info"
"Database Name"
"Server FQDN"
"Provisioning Status"
"Created Date"
"Last Updated Time UTC"
Added support for sqlDatabase connectivity, and listing operations.
Create and delete are in progress, the LRO calls fail