-
Notifications
You must be signed in to change notification settings - Fork 309
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: adds StorageDescriptor and tests #2109
feat: adds StorageDescriptor and tests #2109
Conversation
google/cloud/bigquery/schema.py
Outdated
inputFormat (Optional[str]): Specifies the fully qualified class name of | ||
the InputFormat (e.g. | ||
"org.apache.hadoop.hive.ql.io.orc.OrcInputFormat"). The maximum | ||
length is 128 characters. | ||
locationUri (Optional[str]): The physical location of the table (e.g. | ||
'gs://spark-dataproc-data/pangea-data/case_sensitive/' or | ||
'gs://spark-dataproc-data/pangea-data/'). The maximum length is | ||
2056 bytes. | ||
outputFormat (Optional[str]): Specifies the fully qualified class name | ||
of the OutputFormat (e.g. | ||
"org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat"). The maximum | ||
length is 128 characters. | ||
serdeInfo (Union[SerDeInfo, dict, None]): Serializer and deserializer information. |
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.
inputFormat (Optional[str]): Specifies the fully qualified class name of | |
the InputFormat (e.g. | |
"org.apache.hadoop.hive.ql.io.orc.OrcInputFormat"). The maximum | |
length is 128 characters. | |
locationUri (Optional[str]): The physical location of the table (e.g. | |
'gs://spark-dataproc-data/pangea-data/case_sensitive/' or | |
'gs://spark-dataproc-data/pangea-data/'). The maximum length is | |
2056 bytes. | |
outputFormat (Optional[str]): Specifies the fully qualified class name | |
of the OutputFormat (e.g. | |
"org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat"). The maximum | |
length is 128 characters. | |
serdeInfo (Union[SerDeInfo, dict, None]): Serializer and deserializer information. | |
input_format (Optional[str]): Specifies the fully qualified class name of | |
the InputFormat (e.g. | |
"org.apache.hadoop.hive.ql.io.orc.OrcInputFormat"). The maximum | |
length is 128 characters. | |
location_uri (Optional[str]): The physical location of the table (e.g. | |
'gs://spark-dataproc-data/pangea-data/case_sensitive/' or | |
'gs://spark-dataproc-data/pangea-data/'). The maximum length is | |
2056 bytes. | |
output_format (Optional[str]): Specifies the fully qualified class name | |
of the OutputFormat (e.g. | |
"org.apache.hadoop.hive.ql.io.orc.OrcOutputFormat"). The maximum | |
length is 128 characters. | |
serde_info (Union[SerDeInfo, dict, None]): Serializer and deserializer information. |
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.
Resolved.
google/cloud/bigquery/schema.py
Outdated
self._properties["outputFormat"] = value | ||
|
||
@property | ||
def serde_info(self) -> Union[SerDeInfo, dict, None]: |
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'd never return a dict
though, just SerDeInfo
or None
, right?
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.
resolved.
mypy
sometimes gets confused by a setter that accepts A, B, C
when paired with a getter that can only return A, C.
I added a typing.cast()
call on ~line 680 to help mypy
out and added a comment at that point to explain 'why typing.cast?
'
google/cloud/bigquery/schema.py
Outdated
|
||
prop = _helpers._get_sub_prop(self._properties, ["serDeInfo"]) | ||
if prop is not None: | ||
prop = SerDeInfo("PLACEHOLDER").from_api_repr(prop) |
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.
from_api_repr
should be a class method, so instance shouldn't be required.
prop = SerDeInfo("PLACEHOLDER").from_api_repr(prop) | |
prop = SerDeInfo.from_api_repr(prop) |
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.
Resolved.
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.
Thank you, LGTM.
* feat: adds StorageDescriptor and tests * updates attr names, corrects type hinting
This PR adds the StorageDescriptor class and the associated tests, plus minor tweaks to support both of those changes.