Skip to content
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

[Service Connector] az connection: Support local connection which allow local environment to connect Azure resource #24905

Merged
merged 20 commits into from
Dec 28, 2022

Conversation

xfz11
Copy link
Member

@xfz11 xfz11 commented Dec 14, 2022

Related command

Description

Support local connection, which do some configuration to allow user's local environment to connect some Azure resource, including sql, appconfig, comosdb, mysql, postgresql, etc.

Command group:
az connection
Subgroups:
create : Create a connection from local to a target resource.
preview-configuration : Preview the expected configurations of local connection.
update : Update a Service Connector local connection.

Commands:
delete : Delete a Service Connector local connection.
generate-configuration : Generate configurations of a Service Connector local connection.
list : List local connections of Service Connector.
list-support-types : List client types and auth types supported by local connections.
show : Get the details of a Service Connector local connection.
validate : Validate a Service Connector local connection.
wait : Place the CLI in a waiting state until a condition of the connection is
met.

Command for az connection create group
az connection create appconfig/confluent-cloud/cosmos-cassandra/cosmos-gremlin/cosmos-mongo/cosmos-sql/cosmos-table/eventhub/keyvault/mysql/mysql-flexible/postgres/postgres-flexible/redis/redis-enterprise/servicebus/signalr/sql/storage-blob/storage-file/storage-queue/storage-table/webpubsub
Create connection to connect specific target Azure resource

Testing Guide

History Notes

[Component Name 1] BREAKING CHANGE: az command a: Make some customer-facing breaking change
[Component Name 2] az command b: Add some customer-facing feature


This checklist is used to make sure that common guidelines for a pull request are followed.

@yonzhan
Copy link
Collaborator

yonzhan commented Dec 14, 2022

Service Connector

houk-ms
houk-ms previously approved these changes Dec 21, 2022
Copy link
Contributor

@houk-ms houk-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use [Service Connector] in title for user interface change

Provide more details in PR description, list the commands added in the PR.

@xfz11 xfz11 changed the title {Service Connector} az connection: support local connection [Service Connector] az connection: support local connection Dec 21, 2022
@xfz11 xfz11 changed the title [Service Connector] az connection: support local connection [Service Connector] az connection: Support local connection which allow local environment to connect Azure resource Dec 21, 2022
@xfz11
Copy link
Member Author

xfz11 commented Dec 21, 2022

@kairu-ms Hi Kai, can you help review the PR?

@@ -145,6 +145,7 @@
'PyGithub~=1.38',
'PyMySQL~=1.0.2',
'PyNaCl~=1.5.0',
'pyodbc==4.0.35',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why use "==" instead of "~=" here?

@@ -125,6 +125,7 @@ PyGithub==1.55
PyJWT==2.4.0
PyMySQL==1.0.2
PyNaCl==1.5.0
pyodbc==4.0.35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems more dependencies are included. What's pyodbc used to?

Copy link
Contributor

@kairu-ms kairu-ms Dec 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, I highly recommend moving Service Connector into azure-cli-extension because it involves lots of third parts libraries. And it may block other modules or extensions like az interactive issue

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a valid concern, we will take it into serious consideration and discuss it with the team. While before that, how about we let the new command group in the core to keep consistency.

For the library, if it indeed has some conflicts with other components, i think we can discuss if there are any work arounds there

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bebound is evaluating the pyodbc package.

Copy link
Member Author

@xfz11 xfz11 Dec 22, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kairu-ms and @bebound. As we discussed, pyodbc doesn't support well on linux and it does fail many tests on the pipeline: https://github.com/Azure/azure-cli/pull/24905/checks?check_run_id=10246207156
The homebrew test also failed because it needs to install unixodbc first. The library home page shows the same requirement: https://github.com/mkleehammer/pyodbc
It also says On Windows, the ODBC driver manager is built-in, which means Windows doesn't need prerequisite of installing pyodbc.
So, I remove pyodbc from linux and darwin requirement file and keep the windows requirement file.

@kairu-ms kairu-ms merged commit 71b9d05 into Azure:dev Dec 28, 2022
avgale pushed a commit to avgale/azure-cli that referenced this pull request Aug 24, 2023
…llow local environment to connect Azure resource (Azure#24905)

* support local connection

* recording test

* update test recording

* update help

* fix param help

* update postgres flexible server

* fix help

* fix

* remove enable_pg_extension

* update pyodbc version

* remove pyodbc from setup

* update dependency of pyodbc

* fix credential free

* add test and update help

* update help file

* update credential free to create local users

* update postgres

* fix lint and remove pyodbc dependency

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants