[Core AMQP] move common duplicate internal code in SB & EH back to core-amqp

[ramya-rao-a]

There is quite a bit of common code between Service Bus and Event Hubs which were previously housed in and exported from @azure/core-amqp. But most of these are internal details and need not be exposed to user allowing us to change/break the apis.

As part of v2 for @azure/core-amqp, we are moving such code to Service Bus and Event Hubs packages. This issue is to have a folder to house these changes in a single place and then referenced by Service Bus and Event Hubs packages just like how we have the keyvault-common folder.

Below are the candidates to live in this folder

• Code around SharedKeyCredential and SharedAccessKeyCredential
• validation of connection string when parsing
• Default Data transformer
• Core around EventDataBatch and ServiceBusMessageBatch
• The utility waitForTimeoutOrAbortOrResolve
• Sender that uses the awaitable sender, but whose send() method uses the abortSignal and timers
• The function getRuntimeInfo()

Update as of 15th July 2021:

Based on the below discussions, we want to continue with having core-amqp as the place to share code. Keeping this issue open to move the duplicate code i.e. the candidates mentioned above back to core-amqp

Code pointers:

• Set up your dev env
• Code for @azure/service-bus can be found in sdk/servicebus/service-bus
• Code for @azure/event-hubs can be found in sdk/eventhubs/event-hubs
• Code for @azure/core-amqp can be fond in sdk/core-core-amqp
• Select any of the candidates for duplicate code in the Service Bus and Event Hubs packages to move them to the core-amqp package

[chradek]

I think keeping core-amqp as a package is a better plan than treating it like keyvault-common. I see a couple issues with the keyvault-common approach:

• code duplication in bundles, due to it being present in all packages that consume it
• Some bundlers have issues following the browser mappings correctly using the keyvault-common approach. I ran into this when investigating react-native support. It meant maintaining additional browser mappings that needed to be updated in each package that used the -common approach rather than consolidating the mappings in the package containing the common code (e.g. core-amqp)

[ramya-rao-a]

Thanks @chradek

@xirzec You had some concerns in following the model of keyvault-common. Can you post them here as well?

[xirzec]

Beyond the issues that Chris mentions (size duplication and the difficulty of doing browser/node versions), there are some dev quality of life things (that can perhaps be mitigated by proper configuration):

1. Having long, complex import paths that reach into other folders outside the source root.
2. Because of 1, the output directory structure that TS emits reflects a huge part of the tree, which gets messier to reason about.

[ramya-rao-a]

Based on the above discussions, we want to continue with having core-amqp as the place to share code. Keeping this issue open to move the duplicate code back to core-amqp

[github-actions[bot]]

Hi @ramya-rao-a, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.