fix(DENG-10248): update shredder to support usage reporting derived #8589
Conversation
There was a problem hiding this comment.
I'm not actually sure this test_config does anything or if these changes are even needed / useful.
There was a problem hiding this comment.
The derived tables need to be added to the FakeClient in this file. This should be tested since it's easy to get wrong (as it is now)
There was a problem hiding this comment.
I just took a closer look at the tests and the setup isn't trivial so I can update the tests if you don't want to do it
| field=(USAGE_PROFILE_ID,) * len(sources[table.dataset_id]), | ||
| ): sources[table.dataset_id] | ||
| for table in glean_derived_tables | ||
| if any(field.name == USAGE_PROFILE_ID for field in table.schema) |
There was a problem hiding this comment.
Event though it's unlikely, I suggest adding a check for client_id as well to ensure it doesn't duplicate any table added above.
| if any(field.name == USAGE_PROFILE_ID for field in table.schema) | |
| if any(field.name == USAGE_PROFILE_ID for field in table.schema) | |
| and all(field.name != CLIENT_ID for field in table.schema) |
| field=(USAGE_PROFILE_ID,) * len(sources[table.dataset_id]), | ||
| ): sources[table.dataset_id] |
There was a problem hiding this comment.
This would use the deletion request ping. The derived datasets needs to be added to usage_reporting_sources above
| field=(USAGE_PROFILE_ID,) * len(sources[table.dataset_id]), | |
| ): sources[table.dataset_id] | |
| field=(USAGE_PROFILE_ID,) * len(usage_reporting_sources[table.dataset_id]), | |
| ): usage_reporting_sources[table.dataset_id] |
There was a problem hiding this comment.
The derived tables need to be added to the FakeClient in this file. This should be tested since it's easy to get wrong (as it is now)
fix(DENG-10248): update shredder to support usage reporting derived
This change is to ensure usage_reporting derived datasets get shredded.