[BEAM-2595] Allow table schema objects in BQ DoFn#3556
[BEAM-2595] Allow table schema objects in BQ DoFn#3556sb2nov wants to merge 1 commit intoapache:masterfrom
Conversation
|
Retest this please |
chamikaramj
left a comment
chamikaramj
left a comment
There was a problem hiding this comment.
Thanks. Mostly nits.
| return table_schema | ||
|
|
||
| @staticmethod | ||
| def table_schema_to_dict(table_schema): |
There was a problem hiding this comment.
Please expand this doc comment with info on arg and return value.
| return schema | ||
|
|
||
| @staticmethod | ||
| def get_dict_table_schema(schema): |
| @@ -1192,21 +1192,12 @@ def __init__(self, table_id, dataset_id, project_id, batch_size, schema, | |||
| @staticmethod | |||
| def get_table_schema(schema): | |||
There was a problem hiding this comment.
Please add a doc comment including information about argument and return value.
| elif schema is None: | ||
| if schema is None: | ||
| return schema | ||
| elif isinstance(schema, bigquery.TableSchema): |
There was a problem hiding this comment.
When will we hit this ? Seems like schema will always be a dict now ?
Pls remove if this path is dead.
| @@ -995,6 +1021,45 @@ def test_simple_schema_parsing(self): | |||
| fields=[string_field, number_field]) | |||
| self.assertEqual(expected_table_schema, table_schema) | |||
There was a problem hiding this comment.
Pls expand this test to convert this schema to a dict and compare with the original dict. Also include nested fields.
There was a problem hiding this comment.
Updated with nested fields and added an end-2-end test.
sb2nov
force-pushed
the
BEAM-2595-allow-table-schema-bq-dogn
branch
from
f89d59d to
37ca0db
Compare
sb2nov
left a comment
sb2nov
left a comment
There was a problem hiding this comment.
@chamikaramj PTAL again
| @@ -1192,21 +1192,12 @@ def __init__(self, table_id, dataset_id, project_id, batch_size, schema, | |||
| @staticmethod | |||
| def get_table_schema(schema): | |||
| elif schema is None: | ||
| if schema is None: | ||
| return schema | ||
| elif isinstance(schema, bigquery.TableSchema): |
| return table_schema | ||
|
|
||
| @staticmethod | ||
| def table_schema_to_dict(table_schema): |
| return schema | ||
|
|
||
| @staticmethod | ||
| def get_dict_table_schema(schema): |
| @@ -995,6 +1021,45 @@ def test_simple_schema_parsing(self): | |||
| fields=[string_field, number_field]) | |||
| self.assertEqual(expected_table_schema, table_schema) | |||
There was a problem hiding this comment.
Updated with nested fields and added an end-2-end test.
|
LGTM |
3 participants
Be sure to do all of the following to help us incorporate your contribution
quickly and easily:
[BEAM-<Jira issue #>] Description of pull requestmvn clean verify.<Jira issue #>in the title with the actual Jira issuenumber, if there is one.
Individual Contributor License Agreement.
R: @chamikaramj PTAL