fix(python): add hard-coded keep list for Spanner migration#5210
fix(python): add hard-coded keep list for Spanner migration#5210jskeet merged 2 commits intogoogleapis:mainfrom
Conversation
The "hard-coded keep list" mechanism which was previously only used for Firestore is now generalized to a map from library name to keep list. We may well only find that it's Spanner and Firestore that need this, but it felt like "1 to 2" was the time to generalize. There's no specific test for Spanner, but the existing test for Firestore ensures that the mechanism works, and local testing shows that this keep list is currently sufficient to avoid deleting files during generation.
There was a problem hiding this comment.
Code Review
This pull request refactors the Python migration tool to use a centralized map, pythonExtraKeepLists, for managing library-specific files that should be preserved during migration. This replaces hardcoded logic for the Firestore library and adds a new keep list for the Spanner library. The review feedback recommends using more idiomatic Go by omitting redundant type specifications in the map's composite literals.
JoeWang1127
left a comment
There was a problem hiding this comment.
It would be nice to have a test verified the list is added to the library config.
That would kinda be like having a test for each change to the api.go back when that was mostly data. We've got a test that shows it's added for Firestore; having a Spanner-specific one feels like overkill. Will merge now, but if you feel strongly about it we can revisit it. (I'm hoping to delete this code within 2 weeks though...) |
The "hard-coded keep list" mechanism which was previously only used for Firestore is now generalized to a map from library name to keep list. We may well only find that it's Spanner and Firestore that need this, but it felt like "1 to 2" was the time to generalize.
There's no specific test for Spanner, but the existing test for Firestore ensures that the mechanism works, and local testing shows that this keep list is currently sufficient to avoid deleting files during generation.