Add new dataservices fields #3262
Conversation
| @@ -13,13 +13,13 @@ class DataserviceCsvAdapter(csv.Adapter): | |||
| ("url", lambda d: d.self_web_url()), | |||
| "description", | |||
| "base_api_url", | |||
| "endpoint_description_url", | |||
| "machine_documentation_url", | |||
There was a problem hiding this comment.
It may break the HVD DAG @Pierlou
There was a problem hiding this comment.
As long as it doesn't change anymore I'm fine with it, I'll change it when this is deployed 🤝
| def migrate(db): | ||
| log.info("Processing dataservices…") | ||
|
|
||
| Dataservice.objects(is_restricted=True, has_token=True).update( |
There was a problem hiding this comment.
I don't think we cover all cases since we can have is_restricted and has_token to None.
We should make sure that Dataservice.objects(access_type__exists=True).count() == Dataservice.objects().count() right?
There was a problem hiding this comment.
Hum… We can have None in these fields?
There was a problem hiding this comment.
I did a None request here 6e7c399
udata/core/dataservices/constants.py
Outdated
There was a problem hiding this comment.
Don't forget the changelog explaining the new fields and mentioning the migration
| to_set = {} | ||
| if ( | ||
| dataservice["endpoint_description_url"].endswith(".json") | ||
| or dataservice["endpoint_description_url"].endswith(".yaml") |
There was a problem hiding this comment.
We can add .yml extension as well (see examples).
| or dataservice["endpoint_description_url"].endswith(".yaml") | |
| or dataservice["endpoint_description_url"].endswith(".yaml") | |
| or dataservice["endpoint_description_url"].endswith(".yml") |
| filter={ | ||
| "$or": [ | ||
| { | ||
| "is_restricted": None, | ||
| "has_token": None, | ||
| }, | ||
| {"is_restricted": {"$exists": False}}, | ||
| {"has_token": {"$exists": False}}, | ||
| ] | ||
| }, |
There was a problem hiding this comment.
I don't think you should set all dataservices with None value on is_restricted OR has_token at open.
For example, this dataservice with has_token: null, is_restricted: true is set to open after migration.
I think that None should be treated as False.
You may make this first update to make the rest of the logic easier?
| count = get_db().dataservice.update_many( | ||
| filter={ | ||
| "is_restricted": True, | ||
| "has_token": True, |
There was a problem hiding this comment.
We shouldn't care about has_token here? If we have is_restricted, we can set it to restricted without any worry about the token?
See datagouv/udata-front#655