etalab / entreprise.api.gouv.fr

Site publique d'API Entreprise
https://entreprise.api.gouv.fr
7 stars 7 forks source link

Documentation APIs: disponibilités et privileges de token #69

Closed Haelle closed 3 years ago

Haelle commented 3 years ago

Que fait cette PR ?

Ajout dans la "Documentation Générale" d'une documentation pour les APIs:

Pourquoi fait-on ça ? Contexte, PR, issue liée ?

Il a été demandé lors du dernier OpenLab que les FS/FD puisse avoir accès à la disponibilité par API.

Pour l'API /privileges cette demande vient de Slack

Voilà les screenshots, WDYT ? : Screenshot_2020-10-14 API Entreprise Screenshot_2020-10-14 API Entreprise(1) Screenshot_2020-10-14 API Entreprise(2) Screenshot_2020-10-14 API Entreprise(3)

skelz0r commented 3 years ago

J'aurais attendu d'intégrer cette dispo dans l'API Siade pour le coup.

Du coup je splitterai ton commit en 2 : 1 avec le /privileges (qu'on peut merge direct) et 1 autre avec le availability, où on changera juste l'URL quand ça sera en prod.

Haelle commented 3 years ago

Je vais rajouter aussi /current_status donc.

@skelz0r je ne vois pas ce que tu entends pas "en prod" ? toutes les APIs sont en prod actuellement. Et rien ne va aller dans siade à court terme je pense non ?

Haelle commented 3 years ago

Ajout de /current_status et des corrections (screenshots mis à jour)

skelz0r commented 3 years ago

@skelz0r je ne vois pas ce que tu entends pas "en prod" ? toutes les APIs sont en prod actuellement. Et rien ne va aller dans siade à court terme je pense non ?

J'ai dit ça:

J'aurais attendu d'intégrer cette dispo dans l'API Siade pour le coup.

Ce que je pointe c'est d'intégrer directement dans l'API de Siade et non dans une API à part, pour moi à moyen (court?) terme ça doit être dans Siade et non dans une API à part, du coup là on publierai une documentation avec un tout autre service, qui in-fine va se retrouver dans Siade.

Du coup ce que je proposais c'est de laisser dans une PR tous les endpoints de dispo (lié à watchdoge), de les implémenter dans Siade (quitte a faire tout simplement un proxy en v1 ça suffit largement) et après publier, cela principalement pour éviter d'embrouiller nos consumers avec 2 API distinctes.

Après j'ai envie de dire vu qu'on ne notifie pas à l'heure actuel ces changelogs techniques c'est pas critique de le mettre et changer après, mais vu le coût d'implémentation on peut le faire avant de publier la doc.

cc @brindu

Haelle commented 3 years ago

@skelz0r à discuter parce que je crois qu'on avait justement acté qu'on ne les mettrait pas dans siade

skelz0r commented 3 years ago

@skelz0r à discuter parce que je crois qu'on avait justement acté qu'on ne les mettrait pas dans siade

Si c'est pas dans siade ça devrait être au moins dans le même domaine, niveau développement gérer 2 APIs pour le même service c'est bien relou.

On peut feinter avec du nginx si on veut encore moins se faire chier (reverse proxy sur genre sur le domaine api.entreprise /api/v2/availabilities qui redirige sur le watchdoge.api.entreprise sur api/watchdoge)

skelz0r commented 3 years ago

Cette feinte avec nginx permet de botter en touche sur la question de "est-ce qu'on l'intègre dans siade" du coup, j'aime bien perso.

brindu commented 3 years ago

Si c'est pas dans siade ça devrait être au moins dans le même domaine, niveau développement gérer 2 APIs pour le même service c'est bien relou.

Yes ça je suis d'accord.

J'avais bien en tête de mettre ça hors de SIADE aussi, notamment parce qu'on va proposer de plus en plus d'endpoints de stats dans le futur et j'aurais tendance à trouver ça lourd de tout mettre dans SIADE.

J'aime bien l'idée du hack Nginx aussi.

skelz0r commented 3 years ago

Histoire de clarifier (je pense que j'ai pas été assez clair), je ne parle que de la fonctionnalité qui dit si tel ou tel FD est up ou down (ie le current_status, à voir si on modifie l'interface btw). Les stats dans Siade n'ont pas leur place je suis d'accord.

Fonctionnellement exposer dans siade ce current status (qui serait en fait un health check des FDs) permet aux FS de checker sur les FDs fragiles si ils sont up ou down afin d'éviter de call pour rien et directement répondre "Sorry c'est down chez tel FD".

Du coup mettre dans la doc les endpoints de stats c'est à mon sens du bonus (mais why not maintenant que c'est fait :D)

brindu commented 3 years ago

Ouais je vois ce que tu veux dire, ça fait sens d'avoir les pings au même endroit et ça se hack bien donc ouais ^^

Haelle commented 3 years ago

Je vais faire le hack Nginx et revoir l'interface de /current_status dans la foulée

DorineLam commented 3 years ago

@Haelle J'ai revu la PR, en restructurant une partie du contenu et en harmonisant avec le reste de la doc. Merci pour le gros travail d'agrégation du contenu. Ça m'a beaucoup aidé.

J'ai ajouté un élément pour la troisième API. Il me manque une info, et je voudrai être sûre d'avoir bien compris :

Le paramètre period est personnalisable à souhait en associant les différentes lettre (exemple W pour semaine) avec un chiffre ? Jusqu'à quand l'utilisateur peut-il remonter dans le temps ? Ou autrement dit, combien de temps gardons-nous l’historique dans nos serveurs ? J'avais en tête 6 mois, mais je vois qu'on peut appeler une année dans les exemples.

Haelle commented 3 years ago

Redirection faite. cf Gitlab