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: add AvroOptions
to configure AVRO external data
#994
Conversation
Also: * Unify `ExternalConfig` class to use `_properties` for everything. This does result in more code, but it should make maintenance easier as it aligns with our other mutable resource classes. * Adds `bigtable_options`, `csv_options`, and `google_sheets_options` properties. This aligns with `parquet_options`.
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.
Nothing much to add, except for the type hint duplication nit.
A few lines are missing from the coverage, please just address that.
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.
LGTM. You can address a nit if you want.
"""Additional properties to set if ``sourceFormat`` is set to | ||
GOOGLE_SHEETS. |
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.
(nit) Does this fit into a single line? (IIRC the summary in the docs only grabs the first line)
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.
Now that surprised me, but even better then!
Also:
ExternalConfig
class to use_properties
for everything. This doesresult in more code, but it should make maintenance easier as it aligns with
our other mutable resource classes.
bigtable_options
,csv_options
, andgoogle_sheets_options
properties. This aligns with
parquet_options
.Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
In response to internal request.🦕