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

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

Open
jeremy-holovacs-sp opened this issue Feb 27, 2023 · 7 comments
Assignees
Labels
enhancement The issue is a request for improvement or a new feature question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team

Comments

@jeremy-holovacs-sp
Copy link

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.

@sfc-gh-igarish sfc-gh-igarish added the question Issue is a usage/other question rather than a bug label Mar 30, 2023
@paneerlovr
Copy link

paneerlovr commented Apr 17, 2023

I totally agree with this question...

@julealgon
Copy link

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.

@sfc-gh-dszmolka sfc-gh-dszmolka added the enhancement The issue is a request for improvement or a new feature label Jun 8, 2023
@sfc-gh-dszmolka sfc-gh-dszmolka added the status-triage_done Initial triage done, will be further handled by the driver team label Mar 12, 2024
@ekeroack
Copy link

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

@sfc-gh-dszmolka
Copy link
Contributor

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.

@mungojam
Copy link

mungojam commented May 1, 2024

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?

@ekeroack
Copy link

ekeroack commented May 1, 2024

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

@mungojam
Copy link

mungojam commented May 1, 2024

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.

@sfc-gh-dprzybysz sfc-gh-dprzybysz changed the title Dependencies are tightly coupled, making this difficult to adopt in enterprise applications SNOW-1858345: Dependencies are tightly coupled, making this difficult to adopt in enterprise applications Dec 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement The issue is a request for improvement or a new feature question Issue is a usage/other question rather than a bug status-triage_done Initial triage done, will be further handled by the driver team
Projects
None yet
Development

No branches or pull requests

9 participants