-
Notifications
You must be signed in to change notification settings - Fork 74
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
Splitting up connector code and tests #5466
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
fides Run #11310
Run Properties:
|
Project |
fides
|
Branch Review |
refs/pull/5466/merge
|
Run status |
Passed #11310
|
Run duration | 00m 58s |
Commit |
9a74c12c1d ℹ️: Merge e4b5a404f5ee733106b120ff7ef83f4a49b0c76a into ca8e8470f38218fd95d448e1cf64...
|
Committer | Adrian Galvan |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
View all changes introduced in this branch ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks okay, but there is a method, _generate_table_name() that we havent moved to the snowflake config. It should be moved too, right?
"""Returns field names in clauses surrounded by quotation marks as required by Snowflake syntax.""" | ||
return f'"{string_path}" {operator} (:{operand})' | ||
|
||
def _generate_table_name(self) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're missing this method on the new snowflake query config
fides Run #11343
Run Properties:
|
Project |
fides
|
Branch Review |
main
|
Run status |
Passed #11343
|
Run duration | 00m 55s |
Commit |
b0ef6f2083: Splitting up connector code and tests (#5466)
|
Committer | Adrian Galvan |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
4
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
Closes LA-109
Description Of Changes
Breaking up some large code and test files to make them easier to work with, and to make it easier to see which connectors don't have enough test coverage.
Code Changes
sql_connector.py
into separate files for each connectorquery_config.py
into separate filestest_request_runner_service.py
into separate files for each connector. I left generic request runner service tests in the original file.Steps to Confirm
Pre-Merge Checklist
CHANGELOG.md