Skip to content

deprecate file_metadata stream#96

Merged
vishalp-dev merged 14 commits intomasterfrom
remove-filemetadata-stream
Sep 5, 2024
Merged

deprecate file_metadata stream#96
vishalp-dev merged 14 commits intomasterfrom
remove-filemetadata-stream

Conversation

@vishalp-dev
Copy link
Member

@vishalp-dev vishalp-dev commented Aug 30, 2024

Description of change

  • Deprecated Stream file_metadata
  • Changelog & Version Bump
  • updated integration test to remove field_metadata dependancy in bookmark

Manual QA steps

Risks

Rollback steps

  • revert this branch

Why is file_metadata being removed ?

As per google's latest security updates on Oauth scopes, few scopes are determined as restricted thus requiring us to drop support for the file_metadata stream which utilizes one of the restricted scope.

Specifically: https://www.googleapis.com/auth/drive.metadata.readonly
Reference Document: https://developers.google.com/drive/api/guides/api-specific-auth#scopes

@vishalp-dev vishalp-dev changed the title Update exception handling and sync criteria for file_metadata stream deprecate file_metadata stream Sep 5, 2024
Comment on lines -28 to -30
- Verify no streams are synced when 'file_metadata' bookmark does not change
- Verify that the third sync with the updated simulated bookmark has the same synced streams as the first sync
- Verify that streams will sync based off of 'file_metadata' even when it is not selected
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since file_metadata is now deprecated we don't rely on the file updated timestamp anymore, which was used as a pseudo bookmark earlier, however this does not impact as all the streams are full_table in nature.
removed the extra criteria from the test for the (now) deprecated stream

Copy link
Member

@sgandhi1311 sgandhi1311 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the PR description, it says you have added the exception handling, but looking at the code I don't see the changes anywhere.
Also can you please include the exact reason behind deprecating the file_metadata stream? Including this information would be helpful for future reference.

@sgandhi1311
Copy link
Member

Also please remove the file_metadata occurrences from all the places.
For eg - README.md, state.json.example, all fields test, automatic fields etc.

@vishalp-dev
Copy link
Member Author

updated the PR details and removed references for file_metadata, also updated the tests.

"bookmarks": {
"file_metadata": "2019-09-27T22:34:39.000000Z"
}
"currently_syncing": "sheet_metadata"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include some stream name with the value. The state file looks incomplete.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @sgandhi1311 All the streams are essentially full table so we don't store date specific bookmarks for any stream.

@vishalp-dev vishalp-dev merged commit aa6bac1 into master Sep 5, 2024
@antonio-yuen-zocdoc
Copy link

Hi @sgandhi1311 and @vi6hal , is there any plans on reintroducing something like this? We noticed our hourly syncs more than 10x the amount of data consumed each day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants