Skip to content

feat: dataservices#69

Open
estellebertrand wants to merge 9 commits into
mainfrom
feat/dataservices
Open

feat: dataservices#69
estellebertrand wants to merge 9 commits into
mainfrom
feat/dataservices

Conversation

@estellebertrand

Copy link
Copy Markdown
Contributor

Add the new class API to interact with third-party APIs listed in the data.gouv.fr catalog.

@bolinocroustibat bolinocroustibat left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (or Organization.dataservices), maybe?
  • Could test_client.py also test api()/ create_api() to be consistent with the rest?
  • Could test_organization.py also test APIs in the assertions, to be consistent with the rest?

Comment thread datagouv/api/api.py
from datagouv.utils.retry import simple_connection_retry


class API(BaseObject):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: I would user lowercase Api to comply with Dataset, Topic, Resource etc.

Comment thread datagouv/api/client.py

return DatasetCreator(_client=self).create(payload=payload)

def create_API(self, payload: dict) -> "API":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment thread datagouv/api/client.py
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"]:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should't the typing include Api class as well?

@Pierlou

Pierlou commented Jul 2, 2026

Copy link
Copy Markdown
Collaborator

I guess it has been discussed but maybe the wording Dataservice could be a better option?

  • it sticks to the datagouv API endpoints
  • it prevents confusion with the APIs that are actually at stake here (datagouv's main API and tabular)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants