feat: dataservices#69
Conversation
There was a problem hiding this comment.
Thank you very much for this, this will really change the scope of teh client and the CLI!
A few minor comments, and also two important stuff:
- We should add the documentation about APIs/dataservices in the README as it is already existing for datasets, topics, resources
- There shoud be a
Organization.apis(orOrganization.dataservices), maybe? - Could
test_client.pyalso testapi()/create_api()to be consistent with the rest? - Could
test_organization.pyalso test APIs in the assertions, to be consistent with the rest?
| from datagouv.utils.retry import simple_connection_retry | ||
|
|
||
|
|
||
| class API(BaseObject): |
There was a problem hiding this comment.
NIT: I would user lowercase Api to comply with Dataset, Topic, Resource etc.
|
|
||
| return DatasetCreator(_client=self).create(payload=payload) | ||
|
|
||
| def create_API(self, payload: dict) -> "API": |
There was a problem hiding this comment.
NIT: I would user lowercase create_api for consistency with create_dataservice , api(), etc.
| self.id = id | ||
| self._client = _client | ||
| self._base_metrics_url = ( | ||
| f"https://metric-api.data.gouv.fr/api/{self.__class__.__name__.lower()}s/" |
There was a problem hiding this comment.
Did we test this?
What is the naming used on Metrics API for API/dataservice?
For the API class, that would be ../apis/data/?api_id__exact=..., the expected endpoint is not dataservices?
| cast_as: "Dataset|Organization|Resource|Topic|None" = None, | ||
| ) -> Iterator["Dataset|Organization|Resource|Topic|dict"]: | ||
| cast_as: type["Dataset | Organization | Resource | Topic"] | None = None, | ||
| ) -> Iterator["Dataset | Organization | Resource | Topic | dict"]: |
There was a problem hiding this comment.
Should't the typing include Api class as well?
|
I guess it has been discussed but maybe the wording
|
Add the new class API to interact with third-party APIs listed in the data.gouv.fr catalog.