search

dataforgoodfr / 12_bloom

23 stars 11 forks source link

proposition reogranisation répertoire pour front/back/etl #153

Closed rv2931 closed 4 months ago

rv2931 commented 5 months ago

Proposition de réorganisation pour séparation backend, frontend, etl (à confirmer) et un common (pour les statics entre autres, à appeler directement static éventuellement ou le mettre ailleurs) J'aime bien l'idée de garder la strate "src" pour identifier clairement où se situe le code des différents services et la partie "projet, data, docs, docker ..." => là aussi à confirmer. Vu par ailleurs appeler ce dossier apps plutôt que src

Je propose garder de le front streamlit aussi en historique (pas compliqué de le garder dans un conteneur docker compose dédié) Bien séparer les statics utilisés pour le readm github des statics des services

rv2931 commented 5 months ago

@njouanin @tanega Tant qu'à faire une réorganisation, je sortirais bien tout ce qui est ETL de la partie backend/bloom actuelle dans le sens ou l'ETL n'est pas forcément à être exécuté sur la machine backend (c'est même plutôt déconseillé) et à l'avenir tout ce qui est ETL pourrait se faire avec d'autres outils Pour le moment ça concerne principalement e système de "tasks" que nico a mis en place et que j'ai commencé à structurer un peu plus pour pouvoir faire des pipelines avec gestion des erreurs et tracing qu'en pensez-vous ?

rv2931 commented 5 months ago

@tanega effectivement, compte tenu du fait que ça ne sera pas forcément les mêmes technos entre back et front au final, je trouve pas mal ta proposition d'avoir un répertoire backend et frontend à minima à la racine plutôt que src/backend et src/frontend comme je le proposais. ça permet en plus d'y intégrer les fichiers projets (projecttoml, almebic et autres) au niveau du backend spécifiquement plutôt que de mélanger tout

njouanin commented 5 months ago

OK pour moi. sur la partie streamlit, elle est obsolète, il faudra dégager le code en même temps qu'on supprimera les anciennes tables.

rv2931 commented 5 months ago

Pour le moment j'ai fait un mono-conteneur qui permet de lancer et le serveur back et le service front et qui premettra éventuellement de lancer le service API si on le fait (au moins pour simuler l'API côté front). j'utilise le serveur de dev de Next.JS, je pense pas que ce soit hyper conseillé mais je connais pas trop Next.js donc pour le moment comme on est en dev...

tanega commented 5 months ago

@rv2931 merci pour cette proposition de réorganisation du projet ! En l'état ça me va bien, très bien même ;)

rv2931 commented 4 months ago

ok Je rebase et je resteste ce que je peux et je merge @tanega Je suis pas au clair de comment tu charge tes données pour éventuellement tester la partie front. Tu peux me guider un peu ?

rv2931 commented 4 months ago

Ach .. je suis embêté la réorganisation Backend/Frontend a tout cassé la partie poetry et la partie tox A priori tox aime pas trop avoir des sous répertoires et poetry est configuré pour générer un package bloom et donc avoir un dossier bloom Quand je mets poetry en mode non-package, sur l'idée que ça n'a peut être plus trop de sens de faire un package bloom puisque le front ne sera pas en python, la ça casse les tests d'une autre manière...

rv2931 commented 4 months ago

@njouanin @tanega ça devrait être bon Les tests tox fonctionnent mieux j'ai été moins loin dans le réorg et je n'ai donc pas cassé l'architecture attendue par poetry pour le package bloom Il y le test test_vessel_is_in_port qui fail,

rv2931 commented 4 months ago

Pour un test côté Front

njouanin commented 4 months ago

il doit manquer du nettoyage. Pourquoi on a encore du code dans src/ (et que l'on retrouve également apparemment dans backend/

njouanin commented 4 months ago 
$python3 backend/tasks/facts/extract_spire_data_from_api.py
Traceback (most recent call last):
  File "/Users/nicolasjouanin/Dev/python/bloom/12_bloom/backend/tasks/facts/extract_spire_data_from_api.py", line 7, in <module>
    from bloom.container import UseCases
ModuleNotFoundError: No module named 'bloom.container'
rv2931 commented 4 months ago

il doit manquer du nettoyage. Pourquoi on a encore du code dans src/ (et que l'on retrouve également apparemment dans backend/ il n'y plus rien dans src, je pense que soit il est vide et le dossier n'a pas été supprimé, soit c'est un reste de checkout entre branches https://github.com/dataforgoodfr/12_bloom/tree/folder_reorg_back_front

rv2931 commented 4 months ago

.container'

Ah oui, j'ai oublié de rajouter les PYTHONPATH=${PROJECT_DIR}/backend Désolé, c'est corrigé... j'avais pas testé les tâches :(

njouanin commented 4 months ago

il doit manquer du nettoyage. Pourquoi on a encore du code dans src/ (et que l'on retrouve également apparemment dans backend/ il n'y plus rien dans src, je pense que soit il est vide et le dossier n'a pas été supprimé, soit c'est un reste de checkout entre branches https://github.com/dataforgoodfr/12_bloom/tree/folder_reorg_back_front

OK, il reste effectivement des répertoires dans src, mais vides.

njouanin commented 4 months ago

.container'

Ah oui, j'ai oublié de rajouter les PYTHONPATH=${PROJECT_DIR}/backend Désolé, c'est corrigé... j'avais pas testé les tâches :(

Je ne lance pas depuis docker, mais directement en ligne de commande.

rv2931 commented 4 months ago

Je ne sais plus si le répertoire courant est pris automatiquement dans le pythonpath Si oui faut que je rajoute du backend.bloom... pour tous les imports ou bien un cd ./backend dans la doc du lancement en natif... Ou encore un export /backend avant lancement

Le sam. 20 avr. 2024, 15:39, Nicolas @.***> a écrit :

.container'

Ah oui, j'ai oublié de rajouter les PYTHONPATH=${PROJECT_DIR}/backend Désolé, c'est corrigé... j'avais pas testé les tâches :(

Je ne lance pas depuis docker, mais directement en ligne de commande.

— Reply to this email directly, view it on GitHub https://github.com/dataforgoodfr/12_bloom/pull/153#issuecomment-2067677551, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLDQDUIUGDJ2M6TNVBIPMDY6JVZXAVCNFSM6AAAAABF2IYQEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRXGY3TONJVGE . You are receiving this because you were mentioned.Message ID: @.***>

rv2931 commented 4 months ago

A priori on peut mettre a jour le pythonpath directement dans le code. Je pense que je vais faire ça https://python.doctor/page-python-path-pythonpath

Le sam. 20 avr. 2024, 15:48, Hervé LE BARS @.***> a écrit :

Je ne sais plus si le répertoire courant est pris automatiquement dans le pythonpath Si oui faut que je rajoute du backend.bloom... pour tous les imports ou bien un cd ./backend dans la doc du lancement en natif... Ou encore un export /backend avant lancement

Le sam. 20 avr. 2024, 15:39, Nicolas @.***> a écrit :

.container'

Ah oui, j'ai oublié de rajouter les PYTHONPATH=${PROJECT_DIR}/backend Désolé, c'est corrigé... j'avais pas testé les tâches :(

Je ne lance pas depuis docker, mais directement en ligne de commande.

— Reply to this email directly, view it on GitHub https://github.com/dataforgoodfr/12_bloom/pull/153#issuecomment-2067677551, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADLDQDUIUGDJ2M6TNVBIPMDY6JVZXAVCNFSM6AAAAABF2IYQEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRXGY3TONJVGE . You are receiving this because you were mentioned.Message ID: @.***>

rv2931 commented 4 months ago

@njouanin sinon tu me confirmes que ça fonctionne en te mettant dans le dossier backend ?

njouanin commented 4 months ago

@njouanin sinon tu me confirmes que ça fonctionne en te mettant dans le dossier backend ?

Ni l'un , ni l'autre:

$ python3 backend/tasks/facts/extract_spire_data_from_api.py
Traceback (most recent call last):
  File "/Users/nicolasjouanin/Dev/python/bloom/12_bloom/backend/tasks/facts/extract_spire_data_from_api.py", line 7, in <module>
    from bloom.container import UseCases
ModuleNotFoundError: No module named 'bloom.container'
$ cd backend
$ python3 tasks/facts/extract_spire_data_from_api.py
Traceback (most recent call last):
  File "/Users/nicolasjouanin/Dev/python/bloom/12_bloom/backend/tasks/facts/extract_spire_data_from_api.py", line 7, in <module>
    from bloom.container import UseCases
ModuleNotFoundError: No module named 'bloom.container'
rv2931 commented 4 months ago

@njouanin tu peux réssayer avec tes taches backend/bloom/tasks ? j'avais intégré par erreur les set de tasks backend/tasks que j'ai migré mais dans cette PR c'était pas l'objet et je pense pas qu'elles fonctionne car au final je n'ai pas réorganisé les dossiers comme je l'ai fait la première fois pour compatibilité poetry

njouanin commented 4 months ago

J'ai corrigé les noms de modules (ajout de backend) dans les import. Par contre ça ne fonctionne toujours pas si je n'ajoute pas PYTHONPATH. Ça marchait sans avant ...

rv2931 commented 4 months ago

ne fonctionne toujours pas si je n'ajoute

Moi j'arrive pas à reproduire le problème :( De base pour Python tu dois rajouter le répertoires ou les répertoires de ton projet pour y accéder an tant que module Pour les mêmes raisons que Linux n'exécute pas les exécutables du dossier courant, ce serait une faille de sécurité de charger le module du dossier courant à la place d'un module installé dans les sites-packages de python L'autre solution c'est d'accéder aux modules de ton projet en utiliser le .backend/... en chemin relatif mais idem, il lui faut une référence absolue qui se trouve dans les PYTHONPATH par défaut ou exporté explicitement Donc j'ai tendance à dire que si ça fonctionnait avant sans avoir de variable PYTHONPATH c'est pas tout à fait normal. Maintenant selon les implémentaitons Windows/Linux peut-être qu'il prend le chemin courant automatiquement De mon côté, en faisant un cd backend et python bloom/tasks/load_dim_port... ben,... ça fonctionne, et je comprends pas trop pourquoi. Mais je suis sous Windows

rv2931 commented 4 months ago

Après j'aurais tendance à dire que c'est plus propre de bien scinder les choses à savoir, tu veux travailler dans le backend, tu fais un "cd backend pour que ça fonctionne" idem pour le frontend, sachant qu'il y aura potentiellement un 3ème projet pour la partie interface/API qui oit pouvoir se faire avec une autre techno... ou pas, mais qui doit idéalement être indépendante et du backend et du frontend

njouanin commented 4 months ago

OK, vas-y comme ça. Tu peux merger, c'est bon pour moi.

rv2931 commented 4 months ago

@tanega @njouanin De mon côté la réorg est bonne par contre deux soucis côté github actions:

Je ne sais pas résoudre ces problèmes mais ils ne sont pas liés à la réorg de dossiers