[ServiceBus] Deduplicate parse_connection_string functions

[KieranBrantnerMagee]

Is your feature request related to a problem? Please describe.
from_connection_string is a core component of our APIs, both the ServiceBusClient and ServiceBusAdministrationClient. However, we currently have two distinct implementations, which while very similar are not quite aligned.
(see breadcrumb here)

Describe the solution you'd like
Understand the difference between the two, (Sync with @YijunXieMS if needed for history on why the management client is distinct, if any specific impetus went into that) and unify.

Describe alternatives you've considered
The only real alternative here is keeping them separate. If something is discovered in this process that indicates they should not be unified, am open to that, but would be very curious.

[bradleydamato]

Hey @KieranBrantnerMagee; currently looking at this! If I think it's above my head I'll post again here. @YijunXieMS if you have something on the history of the distinction, feel free to post here!

[bradleydamato]

Still doing some reading and researching @YijunXieMS

I did some pruning of the doc-strings and visual diffing to try and see the core differences here.

Service Bus Client variant

def from_connection_string(
        cls,
        conn_str,
        **kwargs
    ):
        # type: (str, Any) -> ServiceBusClient
     
        host, 
        policy, 
        key, 
        entity_in_conn_str = _parse_conn_str(conn_str)
        
        return cls(
            fully_qualified_namespace=host,
            entity_name=entity_in_conn_str or kwargs.pop("entity_name", None),
            credential=ServiceBusSharedKeyCredential(policy, key), # type: ignore
            **kwargs
        )

Service Bus Administration Client variant

def from_connection_string(
    cls, 
    conn_str: str, 
    **kwargs: Any) -> "ServiceBusAdministrationClient":
        
        endpoint, ###host  in ServiceBusClient
        shared_access_key_name, ###policy  in ServiceBusClient
        shared_access_key, ###key in ServiceBusClient**
        _ ###what looks to be a throwaway of the entity_path
         = parse_conn_str(conn_str)
        
        if "//" in endpoint:
            endpoint = endpoint[endpoint.index("//")+2:]
        
        return cls(
                         endpoint, 
                          ###Notable lack of an entity name 
                         ServiceBusSharedKeyCredential(shared_access_key_name, shared_access_key), 
                          **kwargs)

the biggest diff between the two looks to be the lack of usage of the entity_path (returned var 4) from parse_conn_str in the
Service Bus Administration Client variant ... is that correct in your view?

[YijunXieMS]

@KieranBrantnerMagee @bradleydamato
Frankly speaking I don't remember why there are two parse_conn_str.
By looking back into the history, I guess I found the parse_conn_str in module azure.servicebus._common back then so I used it in the ServiceBusManagementClient(Now renamed to ServiceBusAdministrationClient).
The parse_conn_str in module azure.servicebus._common has been there since the creation of the file on 1/17/2019.

Anyways, I don't think there is anything that prevents the unification of the two parse_conn_str.

[YijunXieMS]

@bradleydamato right, the ServiceBusAdministrationClient doesn't care entity path.

[bradleydamato]

@YijunXieMS @KieranBrantnerMagee do y'all care if I unify, edit the imports, test, and then submit a PR?

[KieranBrantnerMagee]

:) Go right ahead. I will be candid that we're getting a release out the door this friday/monday so my responses may be slightly async until that's locked, but always appreciate the help and would be glad to see this in.

[bradleydamato]

@KieranBrantnerMagee @YijunXieMS PR is here #14228

[bradleydamato]

Hey @KieranBrantnerMagee @YijunXieMS, did the release go out yet?

[yunhaoling]

@bradleydamato , thanks for your contribution!
as the PR has already been merged, closing the issue now.