Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose API on /api/ prefix in prod #51

Merged
merged 5 commits into from
Feb 3, 2022

Conversation

florimondmanca
Copy link
Collaborator

@florimondmanca florimondmanca commented Feb 2, 2022

Refs #46, Préalable à #48

Nginx : actuellement l'API est montée sur la racine /, mais on souhaite que ce chemin soit réservé à l'app client, et que l'API soit accessible depuis l'extérieur sur /api/.

En voici une PR séparée, car la modification est non-triviale. En effet, il faut s'assurer que Nginx et le serveur Python soient bien d'accord, d'où :

  • Passage de Gunicorn à Uvicorn (configuration plus aisée en passant directement par ce dernier, le premier ayant jusqu'ici utilisé son "worker")
  • Mise à jour de la location /api/ dans la config Nginx
  • Option root_path="/api" sur le serveur Uvicorn
  • Correction du proxy_pass pour inclure un / final qui indique à Nginx de retirer /api dans le chemin de la requête transmise à Uvicorn
  • Modification de la redirection /docs pour bien prendre en compte la base_url ainsi définie. (Grâce à ça, Uvicorn renvoie une Location: http://localhost:3579/docs complète, que Nginx interprète correctement via proxy_pass pour le retransformer en http://localhost/api/docs (modif du FQDN et injection du préfixe). Sans ça, on aurait Location: /docs ce que Nginx n'interprèterait pas.)

Pour tester

  • Vérifier que le workflow local est toujours fonctionnel : make serve, accéder au client / à l'API, lancer les tests...
  • J'ai testé de mon côté avec la VM de test :
    • curl localhost renvoie maintenant la page de base Nginx (logique : il n'y a plus de location /)
    • curl localhost/api/ redirige bien vers localhost/api/docs
    • curl localhost/api/datasets/ renvoie bien la liste de datasets

@florimondmanca florimondmanca added infra Ce qui attrait aux environnements de déploiement et à l'infrastructure Cloud refactor Modifications du code sans incidence utilisateur ou fonctionnelle labels Feb 2, 2022
@florimondmanca florimondmanca changed the title Move API to /api/ prefix in prod Expose API on /api/ prefix in prod Feb 2, 2022
@magopian
Copy link
Contributor

magopian commented Feb 3, 2022

En local quand je fais un make serve et que je vais sur http://127.0.0.1:3579 je suis automatiquement redirigé vers http://127.0.0.1:3579/api/docs qui me renvoie (de manière logique j'imagine) {"detail":"Not Found"}.

Est-ce qu'il y a un soucis de redirection vers la doc fastapi ?

@florimondmanca
Copy link
Collaborator Author

@magopian Et je suppose que si tu ajoutes DEBUG=true dans ton .env ça fonctionne ? Selon l'effet de cette ligne : https://github.com/etalab/catalogage-donnees/pull/51/files#diff-c16fbf0c6f7b90a46b94b36f88893c2d174476088608841f7254afba0e81373dR34

Je me demande si je dois ajouter une variable d'environnement sur "l'environnement" justement. Car DEBUG est usuellement utilisé dans le monde Python pour dire "dev", et son absence pour dire "prod" mais évidemment c'est très limité.

Peut-être APP_ENV=prod (et vide / "local" par défaut) ?

@magopian
Copy link
Contributor

magopian commented Feb 3, 2022

Oui tout à fait avec DEBUG=true dans mon .env ça fonctionne correctement 👍

@florimondmanca florimondmanca force-pushed the fm/ops-server-api-prefix branch 2 times, most recently from 4678d5f to 7053a33 Compare February 3, 2022 13:39
@florimondmanca
Copy link
Collaborator Author

florimondmanca commented Feb 3, 2022

@magopian OK, j'ai ajouté un concept de "server mode" qui permet de plus précisément distinguer selon qu'on est en local ou en live (pour désigner une instance de dev, staging, prod, VM de test, etc... tout ce qui serait déployé avec le setup Ansible).

J'ai retesté sur la VM, je crois qu'on est bon.

@florimondmanca florimondmanca removed the refactor Modifications du code sans incidence utilisateur ou fonctionnelle label Feb 3, 2022
@florimondmanca
Copy link
Collaborator Author

@magopian Ça te paraît ✔️ ?

@magopian
Copy link
Contributor

magopian commented Feb 3, 2022

Ça me paraît nickel oui, je me rappelle plus comment on crée le .env au début, peut-être y mettre par défaut la valeur APP_SERVER_MODE=local ?

@florimondmanca
Copy link
Collaborator Author

florimondmanca commented Feb 3, 2022

Le .env en local est conçu comme totalement optionnel, et comme le APP_SERVER_MODE est à local par défaut aussi j'ai pas jugé utile de l'ajouter au .env.example.

Mais si on se dit que ce .env.example est sensé référencer toutes les variables d'env potentiellement utiles, alors OK, je peux l'y rajouter.

@magopian
Copy link
Contributor

magopian commented Feb 3, 2022

ah bah si la valeur par défaut est la bonne, alors c'est nickel ! 🚢

@florimondmanca florimondmanca merged commit a2edb1b into master Feb 3, 2022
@florimondmanca florimondmanca deleted the fm/ops-server-api-prefix branch February 3, 2022 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Ce qui attrait aux environnements de déploiement et à l'infrastructure Cloud
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants