Implementing expand system for commons#19
Conversation
Samuelfaure
force-pushed
the
commons
branch
3 times, most recently
from
e06a0d2 to
3fef1e1
Compare
Samuelfaure
force-pushed
the
commons
branch
2 times, most recently
from
44a7713 to
f724a46
Compare
skelz0r
left a comment
skelz0r
left a comment
There was a problem hiding this comment.
Je pense que je n'ai pas été très clair dans ma proposition: le but du expand.txt est justement de faire des liens symboliques, de bouger le fichier, et rien d'autre.
Si on prends authorizations.yml par exemple, on veut clairement le laisser dans siade/config/ via un lien symbolique, et on laisse le script de déploiement l'expand, i.e. le copier dans config/ au déploiement.
L'objectif était de faire des modifications minimal pour garder du code simple.
|
Pour clarifier ma review, la mécanique est OK, juste le expand.txt n'est pas assez bien utilisé (notamment en prenant l'exemple du |
|
J'ai revert les changements relatifs à authorizations.yml - pour les fichiers swagger ça me semble OK comme ça ? |
skelz0r
left a comment
skelz0r
left a comment
There was a problem hiding this comment.
Je pense que c'est good enough, en lisant je pense qu'on pouvait faire un truc plus intelligent du style:
source:destination
Et du coup par exemple pour authorizations.yml
../commons/authorizations.yml:config/data/authorizations.yml
Et là on aurait eu des changements minimaux et standard dans le code, qui devrait être en vrai la cible de cette PR.
(au passage, j'aurais nommé ça .expand in fine pour éviter d'avoir un .txt)
C'est pas critique, donc tell me si tu veux un approve, mais si tu fais l'effort je suis preneur.
|
Je vais faire l'effort ;D |
--documentation takes way too long
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
SIADE now generates those files in commons/, the single source read by siade/, site/, and mocks/. Each app exposes the files at their original in-app path via a file-level symlink committed in git, so app code keeps reading from its usual path. At deploy time each app's .expand file materializes the real files at those destinations via the source:destination format. site/config/api-particulier-openapi-v3.yml, which was an unused duplicate of commons/swagger/openapi-particulier.yaml on develop, is also converted to a symlink so the site only ever reads one canonical v3 definition.
siade/config/authorizations.yml is now a symlink to commons/data/authorizations.yml, shared with mocks (which previously kept its own divergent copy at mocks/data/siade_authorizations.yml). Mocks generates JWT tokens from this file, so keeping two copies meant scope drift between the real API authorization rules (siade) and the staging tokens (mocks); this is now a single source of truth. - siade/config/authorizations.yml is a symlink to ../commons/data/authorizations.yml so existing code (config_for, generators, etc.) works without changes. - siade/.expand adds commons/data/authorizations.yml:config/authorizations.yml so the deploy script materializes the real file at the in-app path. - mocks/bin/generate_token.rb reads via mocks/commons symlink. - siade docs and CLAUDE.md updated.
Now that siade, site and mocks live in the same repository and share commons/swagger/ (generated by siade), there is no reason to keep the remote-fetch code paths that existed when each app was standalone. site: - AbstractOpenAPIDefinition no longer has remote_url / load_local? / URI.open fallback. Subclasses just declare a local_path Pathname and the abstract class reads it. - APIEntreprise/APIParticulier open-api definition services collapsed to local file reads from their config/api-*.yml paths (which are symlinks to commons/swagger/ in dev, and materialized from commons/ on deploy via .expand). - Removed the four WebMock stubs that faked responses from entreprise.api.gouv.fr and particulier.api.gouv.fr open-api URLs; nothing hits those URLs anymore. - Removed the LOAD_LOCAL_OPEN_API_DEFINITIONS=true invocation from README and CLAUDE. mocks: - lib/open_api_helpers.rb: dropped the ENV['LOCAL'] switch, load_schema_from_remote, the $remote_schemas global, and the open-uri require. load_schema unconditionally reads from commons/swagger/. - README: the paragraph about LOCAL=true is now a statement that commons is the source.
skelz0r
left a comment
skelz0r
left a comment
There was a problem hiding this comment.
Du coup avec le nouveau système et le fait que les liens symboliques existent en local, modulo la modification du système de backend mocks et la suppression des swagger en doublon il n'y a rien à changer non ?
skelz0r
left a comment
skelz0r
left a comment
There was a problem hiding this comment.
Ce qui m'a l'air d'être le cas j'ai mal lu 😅
2 participants
Ajout du systeme + quelques quick-wins de refacto
related https://github.com/datagouv/
testé en sandbox