-
Notifications
You must be signed in to change notification settings - Fork 211
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
fix(types): move JobLoadMetadata writeDisposition #365
fix(types): move JobLoadMetadata writeDisposition #365
Conversation
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!
src/table.ts
Outdated
}; | ||
writeDisposition?: 'WRITE_TRUNCATE'|'WRITE_APPEND'|'WRITE_EMPTY'; |
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 for this. Would you mind taking out the specific string values, however? The upstream is the official resource for acceptable values, and we don't want to duplicate from this layer in the event it is changed upstream.
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.
No problem. I think it improves typing, but I understand your reasoning for wanting to keep it simple.
Keep in mind, I copied the definitions from CopyTableMetadata
which also includes specific string values. 😉
See: https://github.com/googleapis/nodejs-bigquery/blob/master/src/table.ts#L151-L157
I changed the type back to an optional string
and amended/rebased the commit.
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.
Oh, gotcha. I didn't realize it was there, too! Thank you for all of the help!
Codecov Report
@@ Coverage Diff @@
## master #365 +/- ##
=======================================
Coverage 99.44% 99.44%
=======================================
Files 4 4
Lines 544 544
Branches 75 75
=======================================
Hits 541 541
Misses 2 2
Partials 1 1
Continue to review full report at Codecov.
|
The
writeDisposition
field for interfaceJobLoadMetadata
appears to have been defined incorrectly; it does not belong undertimePartitioning
.Typescript fails to compile if I define it under the root, but works as expected if I cast the options to
any
like so:See:
https://cloud.google.com/bigquery/docs/reference/rest/v2/jobs#configuration.load