Fix Pass data and country package versions to APIv2 #2500#2528
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2528 +/- ##
==========================================
+ Coverage 63.62% 63.75% +0.13%
==========================================
Files 56 57 +1
Lines 2161 2213 +52
Branches 287 297 +10
==========================================
+ Hits 1375 1411 +36
- Misses 729 740 +11
- Partials 57 62 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| data_version=( | ||
| uk_dataset_version | ||
| if country_id == "uk" | ||
| else us_dataset_version | ||
| ), |
There was a problem hiding this comment.
issue, blocking: This code is HF-dependent and integrated into API v2
I really do not think we should be adding any code that hitches API v2 to Hugging Face. I think all of the following would be acceptable and a means of getting this over the line:
- Agree that we'll remove dataset version checks after fully migrating (is this a good idea?)
- Agree that we'll write code in a separate PR to check dataset versions within GCP before full migration (probably my preferred way forward)
- Switch to using GCP in API v1 now and write code to check dataset versions from GCP
There was a problem hiding this comment.
resolved: We'll go down pathway 1, removing dataset version checks after fully migrating.
anth-volk
left a comment
There was a problem hiding this comment.
Looks good to me! Holding off on merging until June 4, following completion of regression tests on June 3.
| data_version=( | ||
| uk_dataset_version | ||
| if country_id == "uk" | ||
| else us_dataset_version | ||
| ), |
There was a problem hiding this comment.
resolved: We'll go down pathway 1, removing dataset version checks after fully migrating.
Fixes #2500