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

Add ConnectionString parsing #16782

Closed
nfx opened this issue Feb 17, 2021 · 9 comments · Fixed by #17640
Closed

Add ConnectionString parsing #16782

nfx opened this issue Feb 17, 2021 · 9 comments · Fixed by #17640
Assignees
Labels
Azure.Core blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. P0
Milestone

Comments

@nfx
Copy link

nfx commented Feb 17, 2021

In plenty of cases we receive connection strings for azure resources. We should port ConnectionString parser from iot sdk, so that it's easier and more consistent to work with APIs https://github.com/Azure/azure-iot-sdk-python/blob/master/azure-iot-device/azure/iot/device/common/auth/connection_string.py

@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Feb 17, 2021
@xiangyan99 xiangyan99 added the feature-request This issue requires a new behavior in the product in order be resolved. label Feb 18, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 18, 2021
@xiangyan99 xiangyan99 added Azure.Core Client This issue points to a problem in the data-plane of the library. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Feb 18, 2021
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Feb 18, 2021
@xiangyan99 xiangyan99 removed the question The issue doesn't require a change to the product in order to be resolved. Most issues start as that label Feb 18, 2021
@xiangyan99 xiangyan99 self-assigned this Feb 18, 2021
@xiangyan99 xiangyan99 added this to the Backlog milestone Feb 18, 2021
@xiangyan99
Copy link
Member

Thanks for the feedback. Are you suggesting moving the connection_string parsing into azure.core?

One concern here is different services may have different schemas for the connection string.

e.g. app config service connection string is different from the one for iot. hence _validate_keys may not work correctly for app config.

Could you help give more data points which services can share the parser? That can help us prioritize the work.

Thank you.

@nfx
Copy link
Author

nfx commented Feb 18, 2021

@xiangyan99 i'd mostly need connection string parser for storage and event hubs. that's it :) you can add more services later. typical use case would be getting azure event hub connection string and configuring pyspark consumer with it. or accessing adls gen1/gen2/blob through python sdk directly.

the point is that connection strings are usually the outputs from Terraform & they land in secret stores and stored over there as secret string as a whole. it would be nicer not to write same (even small) amount of code again and again.

you can introduce a ConnectionString class that has generic methods to access keys in a free form and then have subclasses with specific service schema validation. Those subclasses should also extend Credentials class (that usually has SPN client/secret).

@xiangyan99
Copy link
Member

@nfx Thank you for the information. It is very helpful.

@nfx
Copy link
Author

nfx commented Feb 18, 2021

Looking forward to see it and use it :) make sure it propagates to azureeventhubs and storage docs

@swathipil
Copy link
Member

Hi @nfx,

The feature has been added and will be available in the next release. We did consider your proposed option of some sort of ConnectionString class, but ultimately decided that a parsing function is the best solution, as it provides validation that is generic enough for parsing connection strings across services and is simplest to use.

If you are looking for a ConnectionString class, specifically, Event Hubs already has the EventHubConnectionStringProperties class, which I believe is similar to what you are looking for. Storage does not seem to have a similar class, but we can create an issue for this if you still find this necessary.

Thank you for your feedback!

@nfx
Copy link
Author

nfx commented Apr 3, 2021

@swathipil i'd like to see the same consistent ConnectionString class in Python SDK, as there is in Java SDK :) e.g. one standard for all services, because i don't want to learn all different types of connection strings out there :)

@xiangyan99
Copy link
Member

@nfx Could you give us more details what's the difference between the ConnectionString class and dict?

Thank you.

@nfx
Copy link
Author

nfx commented Apr 10, 2021

@swathipil
Copy link
Member

swathipil commented Apr 16, 2021

Hi @nfx, in the core parse_connection_string method, we provide docstring and define the types of the input and type of the keys and values in the output dict. Documentation here.

In the Java example, it looks like there are four instance variables/properties on a ConnectionString, with only one key and one value, rather than parsed key/value pairs. I may be misunderstanding the kind of documentation you are looking for. Would you be able to clarify the purpose of the ConnectionString interface? (If you could provide a sample connection string that would be passed to the implementation of the ConnectionString and the return value of name(), value(), type(), and sticky(), that would be very helpful.)

Thanks!

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Azure.Core blocking-release Blocks release Client This issue points to a problem in the data-plane of the library. customer-reported Issues that are reported by GitHub users external to the Azure organization. feature-request This issue requires a new behavior in the product in order be resolved. P0
Projects
None yet
4 participants