-
Notifications
You must be signed in to change notification settings - Fork 77
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
|
Project |
fides
|
Branch Review |
refs/pull/5466/merge
|
Run status |
|
Run duration | 00m 58s |
Commit |
|
Committer | Adrian Galvan |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
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
|
Project |
fides
|
Branch Review |
main
|
Run status |
|
Run duration | 00m 55s |
Commit |
|
Committer | Adrian Galvan |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
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