Skip to content
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

deprecate file_metadata stream #96

Merged
merged 14 commits into from
Sep 5, 2024
Merged

Conversation

cngpowered
Copy link
Member

@cngpowered cngpowered 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

@cngpowered cngpowered 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.

@cngpowered
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.

@cngpowered cngpowered merged commit aa6bac1 into master Sep 5, 2024
1 check passed
@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