snowflake-connector-net icon indicating copy to clipboard operation
snowflake-connector-net copied to clipboard

SNOW-1858345: Dependencies are tightly coupled, making this difficult to adopt in enterprise applications

Open jeremy-holovacs-sp opened this issue 3 years ago • 11 comments

Dependency list for this project indicates tight coupling with multiple heavy dependencies that can really hamper adoption of this library:

  <ItemGroup>
    <PackageReference Include="AWSSDK.S3" Version="3.7.0.4" />
    <PackageReference Include="Google.Cloud.Storage.V1" Version="2.5.0" />
    <PackageReference Include="Azure.Storage.Blobs" Version="12.13.0" />
    <PackageReference Include="Azure.Storage.Common" Version="12.12.0" />
    <PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
    <PackageReference Include="log4net" Version="2.0.12" />
    <PackageReference Include="Portable.BouncyCastle" Version="1.8.10" />
    <PackageReference Include="System.IdentityModel.Tokens.Jwt" Version="6.10.2" />
    <PackageReference Include="System.Net.Http.WinHttpHandler" Version="6.0.1" />
    <PackageReference Include="System.Text.RegularExpressions" Version="4.3.1" />
  </ItemGroup>
  1. AWSSDK.S3, Google.Cloud.Storage.V1, and Azure.Storage.Blobs should all be abstracted out to an interface and then separate projects (nuget packages) should be made for these implementations. As it is, we are dragging multiple libraries into any project that uses Snowflake that will never be used under almost all circumstances, as almost everyone will just use one of the three libraries. It also prevents a developer from writing their own implementation to handle a less popular storage solution.
  2. Newtonsoft.Json is not the preferred JSON library for modern Microsoft stack. System.Text.Json is a "built-in" version that should be used.
  3. log4net should not be directly referenced; instead use Microsoft.Extensions.Logging.Abstractions which already has a log4net adapter, but also allows the developer to choose other logging mechanisms like Elmah and NLog.
  4. Portable.BouncyCastle is sparsely documented and there are alternatives natively in the MS stack.

My team was reluctant to use this library because it introduced a lot of unnecessary things to our code base. We do want to use the official client, but the risk to our code by adding all of these additional libraries (each of which requires some kind of security review and maintenance) makes us hesitant.

jeremy-holovacs-sp avatar Feb 27 '23 14:02 jeremy-holovacs-sp

I totally agree with this question...

paneerlovr avatar Apr 17 '23 15:04 paneerlovr

I'm also running into challenges to consume this in our large solution. The way this package is built goes against most best practices in terms of dependency management.

Please reconsider the architecture here and split into appropriate sub-packages as needed per dependency in a way that they only need to be referenced when in actual use by the consumer.

julealgon avatar Apr 24 '23 13:04 julealgon

The use of the prerelease package Mono.Unix in 3.0+ makes this especially frustrating. Is there any update on this?

ekeroack avatar Apr 30 '24 12:04 ekeroack

we're already working on decoupling the cloud provider dependencies so that's already in progress, which will partly implement the ask tracked in this ticket, but won't fully resolve it and need multiple iterations.

We're aware how Mono.Unix brings in complications in certain situations and also they do not seem to have a proper release in mind or at least a timeline for it :( .

Thank you for this feedback, the team will consider it when priorizing work needing to be done to fully implement the ask in this Issue.

sfc-gh-dszmolka avatar May 01 '24 06:05 sfc-gh-dszmolka

We're aware how Mono.Unix brings in complications in certain situations and also they do not seem to have a proper release in mind or at least a timeline for it :( .

How come you need any mono reference given the prevalence of .net core that works fine on Linux?

mungojam avatar May 01 '24 06:05 mungojam

We're aware how Mono.Unix brings in complications in certain situations and also they do not seem to have a proper release in mind or at least a timeline for it :( .

How come you need any mono reference given the prevalence of .net core that works fine on Linux?

I imagine .NET Framework?

I'm glad to hear there are plans to remove the Cloud packages from the core package. Are there plans to decouple the Unit tests as well? It seems like that is where the Mono.Unix package is being used though I could be mistaken

ekeroack avatar May 01 '24 14:05 ekeroack

We're aware how Mono.Unix brings in complications in certain situations and also they do not seem to have a proper release in mind or at least a timeline for it :( .

How come you need any mono reference given the prevalence of .net core that works fine on Linux?

I imagine .NET Framework?

Could be, in that case the libraries can be changed to .net standard and then make the unit tests run in the GitHub actions under a windows runner for .net framework testing. I haven't looked into this repo yet but might try to at some point.

mungojam avatar May 01 '24 19:05 mungojam