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

Make dependencies optional #125

Open
qais-cisco opened this issue Sep 1, 2022 · 7 comments
Open

Make dependencies optional #125

qais-cisco opened this issue Sep 1, 2022 · 7 comments
Labels
enhancement New feature or request

Comments

@qais-cisco
Copy link

qais-cisco commented Sep 1, 2022

Driver version

2.0.908

Problem description

In order to use the connecter, we have to pull in dependencies we don't need:

beautifulsoup4
lxml
scramp
soupsieve

This should be optional and not a requirement.

@Brooke-white
Copy link
Contributor

Hi @qais-cisco, thanks for opening this issue. Could you please share which dependencies you are referring to?

@qais-cisco
Copy link
Author

@Brooke-white , updated the issue with the dependencies.

@Brooke-white
Copy link
Contributor

@qais-cisco , to give some context these dependencies are used in the following ways:

  • scramp: here for authentication with redshift
  • lxml, beautifulsoup4, soupsieve: here + all of the IdentityProviderPlugin code (found in plugin folder) use beautifulsoup for parsing HTML and XML (which uses lxml as underlying parser) during the Identity provider Plugin authentication process

Removing lxml,beautifulsoup4, and soupsieve would be a breaking change for our current users and not provide good user experience for new users utilizing IdentityProviderPlugins.

As previously discussed here, lxml is the largest sized dependency of the bunch. A potential path for migrating away from lxml to the Python standard library parser would reduce the package size, but needs to be investigated first.

I started looking into this and found the IdentityProviderPlugin code relating to authentication would need some rework and more testing. If this is something you/anyone else would be willing to dig into I'm more than happy to support.

@mjschultz
Copy link

not provide good user experience for new users utilizing IdentityProviderPlugins.

To be clear, I'm a new user to this library and I don't like the experience of having to bring in several third party dependencies for authentication protocols I'm not going to use. I could uninstall them after that fact, but that introduces weird timing dependencies outside the normal requirements.txt flow.

Python's setup.py supports extras_require so it's just a matter of moving those dependencies into that section, allowing users that use the non-plugin or SASL methods to not have to bring those in (yes, this means current users would have to change their requirements to become redshift_connector[sasl]>=3.0.0 going forward or redshift_connector[saml]>=3.0.0 to bring in the appropriate deps. But that's what versioning (and documentation) is for!

All the BS4 oriented code looks like it lazily imports those libraries already, so it shouldn't require any real code changes either. I'm not sure if the SASL/scramp auth method is the most common or not, but that code could be lazily imported as well unless it's super common.

@Brooke-white
Copy link
Contributor

Thanks for the feedback, @mjschultz. I will discuss with the redshift_connector product team

@Brooke-white
Copy link
Contributor

I've added this item to our roadmap but no clear ETA at this time. Will provide updates here as needed.

@Brooke-white Brooke-white added the enhancement New feature or request label Apr 27, 2023
@TonySherman
Copy link

I would also be interested in this feature. I am using this in a lambda function and it would be nice if I did not have to include the boto3/botocore libraries as well as the other large dependencies mentioned here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants