snowflakedb / snowflake-connector-nodejs

NodeJS driver
Apache License 2.0
122 stars 130 forks source link

SNOW-1650373: Too many dependencies #906

Closed DScheglov closed 2 months ago

DScheglov commented 2 months ago

What is the current behavior?

Installing the snowflake-sdk on the empty project brings ~305 unique dependencies. Installing it to our existing project +160 (plus around 15% to what we have now).

Do we realy need smithy, aws/s3-client, moment!!, axios!! In the driver???

What is the desired behavior?

It is better to minimize 3rd party dependencies to less then 10. Ideally it must not have any 3rd party dependency

How would this improve snowflake-connector-nodejs?

Currently we have to consider using another DB, because Snowflake doesn't have a trustable (I really cannot understand why I need smithy in the driver) nodejs-driver.

References, Other Background

Please consider this comparision:

https://gist.github.com/DScheglov/edfc385c4467abaf300ca56f06a8aee3

sfc-gh-dszmolka commented 2 months ago

we're tracking the request of removing cloud dependencies at https://github.com/snowflakedb/snowflake-connector-nodejs/issues/449

axios will stay for a while, being the main workhorse for http communication for the moment.

closing it as a duplicate.

DScheglov commented 2 months ago

@sfc-gh-dszmolka , with all respect.

Are moment and moment-timezone cloud-specific dependenecies?

Or fast-xml-parser is?

We already have xml2js, why do we need a new one? Actually, xml-parser is not an easy to be added to the project, beause it is a potential source of vulnarabilities (more then other npm packages). And actually we don't have an xml fields, so why do we need this dependency at all? (we need xml2js to work with SAML).

Or winston? Why does driver install winston? And yes, is winston also a cloud-specific dependency?

It seems the driver does a lot of work on behalf of its clients from one side and has a ... low level API from other side. Perhaps it makes sense to change the balance on the counter side: make convinient API and make customization through the injection, doesn't it?

So, I don't consider this issue as a duplicated for cloud-specific one.

And yes, unfortunatelly we cannot consider this driver until it depends on axios and moment (because it will be another package when this dependencies will be removed, so it will require to be reviewed again before including to the project).

sfc-gh-dszmolka commented 2 months ago

thank you @DScheglov for taking the time to describe your concern in detail. While one could agree with all of them, you also need to consider that certain parts of the library come from 6 years ago and well, the industry and best practices changed in the meantime so you're right to expect that every library follows the same trends and the library is constantly refactored and optimized. Or even the whole approach, which worked 6 years ago, reconsidered and reimplemented, maybe with 10 dependencies which better fits today's trends. Maybe Promises instead of callbacks, and so on. There are a lot of elements to consider and refactor.

It is always possible to improve the driver, and we always need to decide how we allocate the resources who needs to be divided between your request and all the others. This is of course not your problem. That's why I ask you to kindly contact your Snowflake account representative and explain to them how this request is important (or even a blocker!) to your use case. They can then directly work with the product team to help prioritize amongst all the other requests.

Alternatively if you have time and possibility, you can also submit PR which then could be reviewed by the team and we thank you for your contribution in advance!

Even more alternatively, perhaps you could also take a look at Snowflake's SQL REST API - depending on your use-case, maybe it even better fits your requirements as you can directly talk to Snowflake without a driver, with a http library of your choice without any other dependencies. (at the cost of some capabilities, which you might or might not even use)

DScheglov commented 2 months ago

@sfc-gh-dszmolka

is this driver just a wrapper around this SQL REST API? Or it also supports other interfaces?

That's why I ask you to kindly contact your Snowflake account representative and explain to them how this request is important (or even a blocker!) to your use case.

Already done! ) I will look at the SQL REST API and perhaps we can use it. Otherwise we will fallback to postgress