-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Add annotations to the code #1982
Changes from 3 commits
2f17424
ca45436
d4c1dbb
9975cd5
43213cd
5a929b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,5 @@ | ||||||||||
import agate | ||||||||||
from typing import Any, Optional, Tuple, Type | ||||||||||
from typing import Any, Optional, Tuple, Type, List | ||||||||||
|
||||||||||
import dbt.clients.agate_helper | ||||||||||
from dbt.contracts.connection import Connection | ||||||||||
|
@@ -9,6 +9,7 @@ | |||||||||
from dbt.adapters.sql import SQLConnectionManager | ||||||||||
from dbt.logger import GLOBAL_LOGGER as logger | ||||||||||
|
||||||||||
from core.dbt.adapters.factory import BaseRelation | ||||||||||
|
||||||||||
LIST_RELATIONS_MACRO_NAME = 'list_relations_without_caching' | ||||||||||
GET_COLUMNS_IN_RELATION_MACRO_NAME = 'get_columns_in_relation' | ||||||||||
|
@@ -38,6 +39,7 @@ class SQLAdapter(BaseAdapter): | |||||||||
- list_relations_without_caching | ||||||||||
- get_columns_in_relation | ||||||||||
""" | ||||||||||
|
||||||||||
ConnectionManager: Type[SQLConnectionManager] | ||||||||||
connections: SQLConnectionManager | ||||||||||
|
||||||||||
|
@@ -63,32 +65,32 @@ def add_query( | |||||||||
abridge_sql_log) | ||||||||||
|
||||||||||
@classmethod | ||||||||||
def convert_text_type(cls, agate_table, col_idx): | ||||||||||
def convert_text_type(cls, agate_table: agate.Table, col_idx: int) -> str: | ||||||||||
return "text" | ||||||||||
|
||||||||||
@classmethod | ||||||||||
def convert_number_type(cls, agate_table, col_idx): | ||||||||||
def convert_number_type(cls, agate_table: agate.Table, col_idx: int) -> str: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ugh, I always forget this - our pylint is set to 79 chars, not 80, so this line (just barely) fails it.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Whoops, just pushed a fix. |
||||||||||
decimals = agate_table.aggregate(agate.MaxPrecision(col_idx)) | ||||||||||
return "float8" if decimals else "integer" | ||||||||||
|
||||||||||
@classmethod | ||||||||||
def convert_boolean_type(cls, agate_table, col_idx): | ||||||||||
def convert_boolean_type(cls, agate_table: agate.Table, col_idx: int) -> str: | ||||||||||
return "boolean" | ||||||||||
|
||||||||||
@classmethod | ||||||||||
def convert_datetime_type(cls, agate_table, col_idx): | ||||||||||
def convert_datetime_type(cls, agate_table: agate.Table, col_idx: int) -> str: | ||||||||||
return "timestamp without time zone" | ||||||||||
|
||||||||||
@classmethod | ||||||||||
def convert_date_type(cls, agate_table, col_idx): | ||||||||||
def convert_date_type(cls, agate_table: agate.Table, col_idx: int) -> str: | ||||||||||
return "date" | ||||||||||
|
||||||||||
@classmethod | ||||||||||
def convert_time_type(cls, agate_table, col_idx): | ||||||||||
def convert_time_type(cls, agate_table: agate.Table, col_idx: int) -> str: | ||||||||||
return "time" | ||||||||||
|
||||||||||
@classmethod | ||||||||||
def is_cancelable(cls): | ||||||||||
def is_cancelable(cls) -> bool: | ||||||||||
return True | ||||||||||
|
||||||||||
def expand_column_types(self, goal, current): | ||||||||||
|
@@ -114,7 +116,7 @@ def expand_column_types(self, goal, current): | |||||||||
|
||||||||||
self.alter_column_type(current, column_name, new_type) | ||||||||||
|
||||||||||
def alter_column_type(self, relation, column_name, new_column_type): | ||||||||||
def alter_column_type(self, relation, column_name, new_column_type) -> None: | ||||||||||
""" | ||||||||||
1. Create a new column (w/ temp name and correct type) | ||||||||||
2. Copy data over to it | ||||||||||
|
@@ -158,13 +160,13 @@ def rename_relation(self, from_relation, to_relation): | |||||||||
kwargs=kwargs | ||||||||||
) | ||||||||||
|
||||||||||
def get_columns_in_relation(self, relation): | ||||||||||
def get_columns_in_relation(self, relation: str): | ||||||||||
return self.execute_macro( | ||||||||||
GET_COLUMNS_IN_RELATION_MACRO_NAME, | ||||||||||
kwargs={'relation': relation} | ||||||||||
) | ||||||||||
|
||||||||||
def create_schema(self, database, schema): | ||||||||||
def create_schema(self, database: str, schema: str) -> None: | ||||||||||
logger.debug('Creating schema "{}"."{}".', database, schema) | ||||||||||
kwargs = { | ||||||||||
'database_name': self.quote_as_configured(database, 'database'), | ||||||||||
|
@@ -173,7 +175,7 @@ def create_schema(self, database, schema): | |||||||||
self.execute_macro(CREATE_SCHEMA_MACRO_NAME, kwargs=kwargs) | ||||||||||
self.commit_if_has_connection() | ||||||||||
|
||||||||||
def drop_schema(self, database, schema): | ||||||||||
def drop_schema(self, database: str, schema: str) -> None: | ||||||||||
logger.debug('Dropping schema "{}"."{}".', database, schema) | ||||||||||
kwargs = { | ||||||||||
'database_name': self.quote_as_configured(database, 'database'), | ||||||||||
|
@@ -182,7 +184,7 @@ def drop_schema(self, database, schema): | |||||||||
self.execute_macro(DROP_SCHEMA_MACRO_NAME, | ||||||||||
kwargs=kwargs) | ||||||||||
|
||||||||||
def list_relations_without_caching(self, information_schema, schema): | ||||||||||
def list_relations_without_caching(self, information_schema, schema) -> List[BaseRelation]: | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is too long and will end up failing pylint, it's too many characters! There are a other more lines like this too (119, 81, 77, 72)
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks, I've set the line length to 80. |
||||||||||
kwargs = {'information_schema': information_schema, 'schema': schema} | ||||||||||
results = self.execute_macro( | ||||||||||
LIST_RELATIONS_MACRO_NAME, | ||||||||||
|
@@ -209,18 +211,18 @@ def list_relations_without_caching(self, information_schema, schema): | |||||||||
)) | ||||||||||
return relations | ||||||||||
|
||||||||||
def quote(cls, identifier): | ||||||||||
def quote(self, identifier): | ||||||||||
return '"{}"'.format(identifier) | ||||||||||
|
||||||||||
def list_schemas(self, database): | ||||||||||
def list_schemas(self, database: str) -> List[str]: | ||||||||||
results = self.execute_macro( | ||||||||||
LIST_SCHEMAS_MACRO_NAME, | ||||||||||
kwargs={'database': database} | ||||||||||
) | ||||||||||
|
||||||||||
return [row[0] for row in results] | ||||||||||
|
||||||||||
def check_schema_exists(self, database, schema): | ||||||||||
def check_schema_exists(self, database: str, schema: str) -> bool: | ||||||||||
information_schema = self.Relation.create( | ||||||||||
database=database, | ||||||||||
schema=schema, | ||||||||||
|
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.
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.
What's the difference between core and non-core?
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.
dbt makes use of "namespace packages", as defined in PEP 420. The folder
core
contains one root namespace package in thedbt
namespace, which you can install asdbt-core
. The various plugin folders also contain root namespace packages in thedbt
namespace - in particular they modifydbt.include
anddbt.adapters
by adding appropriate entries there.So
core
is part of the folder structure and doesn't have any semantic meaning on the python side.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.
Thanks for the explanation, appreciate it.