-
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
feat: partial metadata projection #1258
Conversation
…t_partial_metadata_projection updating changes on main branch
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.
Checking the implementation guide, I think a good idea to test the different between getting a basic view vs full view is to check for the lastModifiedTime
and/or numBytes
, as in the basic view, those attributes are not computed by the backend.
system-test/bigquery.ts
Outdated
@@ -888,9 +887,25 @@ describe('BigQuery', () => { | |||
const description = 'catsandstuff'; | |||
await table.setMetadata({description}); | |||
const [metadata] = await table.getMetadata(); | |||
const metadataProps = Object.values(metadata); | |||
assert.strictEqual(metadataProps.length, 18); |
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.
I think checking for the number of attributes here can lead the test to be flaky in the long term, as every time that the service team adds a new field, the test is going to break.
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.
Good point!
system-test/bigquery.ts
Outdated
Object.keys(basicMetadata.metadata) | ||
); | ||
|
||
assert.strictEqual(basicMetadataProps.length, 10); |
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.
Same comment around testing for the amount of fields
I'm already checking against |
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:
This PR updates the documentation to specify how to get a partial metadata projection via the
table.get()
method, and clarified the difference between this method andtable.getMetadata()
in terms of being selective about metadata retrieved. A system test for this usage oftable.get()
has been added, as well as an expansion of thetable.getMetadata()
system test to help illustrate the differences between these two methods. This PR reflects an update to the BigQuery API across all languages, not a GH issue for this repo.