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

feat: Add config key for connect_arg for SqlAlchemyExtractor #434

Merged

Conversation

benrifkind
Copy link
Contributor

Summary of Changes

I added a config key for the SQLAlchemyExtractor that lets the user specify general connect_args for the sqlalchemy.create_engine function. I did this because I want to use the SQLAlchemyExtractor for Presto and I have an https connection so I need to pass the kwarg connect_args={"protocol": "https"} into create_engine.

Tests

I added the test: test_get_connection in tests/unit/extractor/test_sql_alchemy_extractor.py

Documentation

I did not add any documentation

CheckList

Make sure you have checked all steps below to ensure a timely review.

  • PR title addresses the issue accurately and concisely. Example: "Updates the version of Flask to v1.0.2"
  • PR includes a summary of changes.
  • PR adds unit tests, updates existing unit tests, OR documents why no test additions or modifications are needed.
  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
  • PR passes make test

Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind benrifkind force-pushed the benrifkind-sqlalchemy-connect-args branch from e372d63 to 205a96f Compare February 3, 2021 22:23
Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind benrifkind force-pushed the benrifkind-sqlalchemy-connect-args branch from ea8745e to 538038d Compare February 3, 2021 22:37
@feng-tao
Copy link
Member

feng-tao commented Feb 3, 2021

looks good

@feng-tao
Copy link
Member

feng-tao commented Feb 4, 2021

hey @benrifkind , do you think you could fix the DCO check by running git commit --amend --no-edit --signoff ? thanks.

Signed-off-by: benrifkind <ben.rifkind@gmail.com>
@benrifkind benrifkind force-pushed the benrifkind-sqlalchemy-connect-args branch from 0daf9fd to 3bcf14b Compare February 4, 2021 02:20
@benrifkind
Copy link
Contributor Author

Sorry about that. I fixed the DCO check.

@feng-tao
Copy link
Member

feng-tao commented Feb 4, 2021

no worry

@feng-tao feng-tao merged commit 7f3be0f into amundsen-io:master Feb 4, 2021
@benrifkind benrifkind deleted the benrifkind-sqlalchemy-connect-args branch February 4, 2021 16:00
Wonong pushed a commit to Wonong/amundsendatabuilder that referenced this pull request Mar 4, 2021
…n-io#434)

* General connect_args for SqlAlchemyExtractor

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* lint

Signed-off-by: benrifkind <ben.rifkind@gmail.com>

* more lint

Signed-off-by: benrifkind <ben.rifkind@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants