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.

Sign up for GitHub

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

Jump to bottom

Update dev_requirements.txt and THIRD_PARTY_LICENSES.txt for sqlparse… #181

Merged
merged 2 commits into from
May 5, 2023
Merged

Update dev_requirements.txt and THIRD_PARTY_LICENSES.txt for sqlparse… #181

merged 2 commits into from
May 5, 2023

Conversation

ArgusLi
Copy link
Contributor

@ArgusLi ArgusLi commented May 3, 2023

… v0.4.4.

Summary

Upgrade sqlparse to v0.4.4

Description

Upgrade sqlparse to v0.4.4 to fix security vulnerability

Test Results

Smoke tests passed successfully.

Changelog

  • Added a summary of what this PR accomplishes to CHANGELOG.md

@ArgusLi ArgusLi requested review from ravjotbrar and jlarue26 May 3, 2023 19:13
@ArgusLi ArgusLi self-assigned this May 3, 2023
@ArgusLi ArgusLi linked an issue May 3, 2023 that may be closed by this pull request
Upgrade sqlparse to 0.4.4 #180
Closed
ravjotbrar
ravjotbrar reviewed May 3, 2023
View reviewed changes
Copy link
Contributor

@ravjotbrar ravjotbrar left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need sqlparse as a requirement? I wonder how many of these packages we could remove.

@ArgusLi
Copy link
Contributor Author

ArgusLi commented May 3, 2023

Do we need sqlparse as a requirement? I wonder how many of these packages we could remove.

dbt-core requires sqlparse - this can be seen here.

@ravjotbrar
Copy link
Contributor

dbt-core requires sqlparse - this can be seen here.

Those requirements in the setup.py file specify all the packages that will be downloaded when you install dbt-core, which means sqlparse will automatically be downloaded with the installation of dbt-core.

Regardless, our dev_requirements do not impact our adapter's functionality, only development (such as testing). Here is an example of snowflake's dev requirements: https://github.com/dbt-labs/dbt-snowflake/blob/main/dev-requirements.txt

@ArgusLi
Copy link
Contributor Author

ArgusLi commented May 4, 2023

You raise a good point, a lot of the dependencies are taken from dbt-core. We should try and remove as many dependencies from dev_requirements as possible.

ravjotbrar
ravjotbrar approved these changes May 5, 2023
View reviewed changes
Copy link
Contributor

@ravjotbrar ravjotbrar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, but will need to revisit cleaning up dev requirements in the future.

@ArgusLi ArgusLi merged commit e73995b into main May 5, 2023
@ArgusLi ArgusLi deleted the 180-upgrade-sqlparse-to-044 branch May 5, 2023 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ravjotbrar ravjotbrar ravjotbrar approved these changes

@jlarue26 jlarue26 Awaiting requested review from jlarue26

Assignees

@ArgusLi ArgusLi

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade sqlparse to 0.4.4
2 participants
@ArgusLi @ravjotbrar