Doapi metadata enrichment #1575
Conversation
URL hardcoded, api_key hardcoded, everything hardcoded, but it works.
…m-download-of-bq-datasets Rtorre/ch56013/client for stream download of bq datasets
And refactor a little: move all the client details to _BQDatasetClient and keep all the schema-related info in BQUserDataset.
Note it doesn't support f-strings
…ts for its completion
…taset-creation Rtorre/ch56009/client for bq dataset creation
…enrichment-id-already-exists Fix for duplicate/wrong columns when reusing enrichment object
…en-running-catalog-subscriptions Add type filter to get_subscription_ids
|
|
||
| _DATASET_ID_FIELD = 'id' | ||
| _DATASET_SLUG_FIELD = 'slug' | ||
| _ALLOWED_FILTERS = [CATEGORY_FILTER, COUNTRY_FILTER, GEOGRAPHY_FILTER, PROVIDER_FILTER, VARIABLE_FILTER] | ||
| _ALLOWED_FILTERS = [CATEGORY_FILTER, COUNTRY_FILTER, GEOGRAPHY_FILTER, PROVIDER_FILTER] |
There was a problem hiding this comment.
Why is VARIABLE_FILTER removed here?
There was a problem hiding this comment.
There was a problem hiding this comment.
I don't remember the reason. Not sure if we decided that makes no sense... but better to know the @juanrmn opinion
There was a problem hiding this comment.
It's been a time since that, but I think I removed that because dataset's does not have the variable attribute, so if I'm not wrong, it would have failed with the previous code anyway.
Also, to include this filter in the DO API, it would need a quite heavy join between datasets and variables, I think. But please correct me if I'm wrong.
There was a problem hiding this comment.
OK then. Let's keep it as it is and change it later if necessary.
cartoframes/data/observatory/catalog/repository/geography_repo.py
Outdated
Show resolved
Hide resolved
|
About DODataset e2e tests. I would move |
Yes, I agree. For now, It is better to have it here than not, but we should do it there. Some problems I found and coming to my mind now, were:
|
nope, this has long been discussed. If they are end-to-end, they shall be here. The API has its own tests (service component) The client has its own tests (it abides to the API) Here's the missing piece that should save time testing manually (and already did) |
|
@oleurud there is a plan to deprecate py27 in carto-python. We could tackle the test suite implementation then. @rafatower usually libraries contain their own integration tests (e2e). That's what we have in CARTO.js or CARTO VL, for example. But, as I said, it's no big deal to keep them here for now. |
This is just to trigger CI