Pilotage: Partage du fichier import fluxIAE via nouveau bucket S3 #5249

calummackervoy · 2024-12-11T16:01:47Z

🤔 Pourquoi ?

On remplace la commande populate_metabase_flux_iae avec un DAG géré au côté Pilotage. Avec ce PR, on supprime ce cher processus en le remplaçant avec une commande plus simple à téléversé l'archive vers un S3 accessible par le Pilotage.

🚨 À vérifier

Mettre à jour le CHANGELOG_breaking_changes.md ?

rsebille

Je te propose un plan en 3 PR/étapes

Étape 1 - PR emplois : Téléversement du flux IAE

PR avec juste un commit qui ajoute (sans toucher à l'existant) une tâche cron qui copie les archives ZIP FluxIAE inexistantes du dossier asp_riae_shared_bucket vers le bucket pilotage-datastore-les-emplois.

Comme ça une fois fusionnée on aura les archives FluxIAE de disponibles dans le bucket pour tout le monde, tout le temps.

Étape 2 - PR pilotage : DAG flux IAE

Avec l'étape 1 de faite, la PR devient plus facilement testable car les fichiers sont disponible sur le S3, et aussi pas besoin de grosse configuration à faire pour la faire fonctionner sur notre local dev ou sur la prod.

Truc auquel je pense dès à présent c'est que c'est le DAG qui devra choisir l'archive à utiliser, je pense que tu peux reprendre la logique que j'ai faite pour l'import des EA/EATT et qui se base aussi sur une archive hebdomadaire.

Étape 3 - PR emplois : Nettoyage de `populate_metabase_flux_iae`

C'est grosso modo le commit initial de la PR actuelle où on vire tout ce qui lui est lié car maintenant c'est géré par un DAG.

itou/metabase/dataframes.py

config/settings/base.py

itou/metabase/management/commands/upload_fluxiae_to_s3.py

scripts/import-iae.sh

itou/metabase/management/commands/upload_fluxiae_to_s3.py

calummackervoy · 2024-12-12T17:53:29Z

clevercloud/cron.json

@@ -31,6 +31,7 @@
  "0 0 * * 1 $ROOT/clevercloud/run_management_command.sh shorten_active_sessions",
  "0 2 * * 1 $ROOT/clevercloud/crons/populate_metabase_matomo.sh",
  "0 12 * * 1 $ROOT/clevercloud/run_management_command.sh import_ea_eatt --from-asp --wet-run",
+  "0 12 * * 1 $ROOT/clevercloud/upload_to_pilotage.sh",


12h chaque lundi - j'ai compris que c'est hebdomanaire mais j'imagine il faudra changer l'heure ?

Je pense que midi c'est bien, on changera si on rencontre des problèmes.
A noter que l'heure système des machines Clever est en UTC et que l'ASP est en heure de métropole (CET/CEST suivant la saison) donc on devrait être bon, et avoir en plus de la marge :).

... Dec 2 11:14 fluxIAE_ITOU_20241202_115450609.tar.gz ... Dec 9 09:48 fluxIAE_ITOU_20241209_102749998.tar.gz ... Dec 16 09:51 fluxIAE_ITOU_20241216_103042036.tar.gz

calummackervoy · 2024-12-12T17:54:23Z

clevercloud/upload_to_pilotage.sh

+    exit 0
+fi
+
+/bin/bash $ROOT/clevercloud/run_management_command.sh upload_data_to_pilotage "$FLUX_IAE_FILE"


Je n'ai pas testé sur prod. On peut faire un dry-run sur une recette jetable ?

(J'appele run_management_command.sh depuis ce script pour minimiser la duplication de code)

Je n'ai pas testé sur prod. On peut faire un dry-run sur une recette jetable ?

Oui tu pourras tester sur une recette jetable, ceci devrais fonctionner :

Tu te connectes en SSH sur ta recette jetable

Puis mkdir "$APP_HOME/asp_riae_shared_bucket && touch "$APP_HOME/asp_riae_shared_bucket/fluxIAE_ITOU_$(date '+%Y%m%d_%H%M%S').tar.gz" pour créer un dummy file

Tu lances ton script et vérifie que le fichier est bien reçu dans le bucket S3 pilotage-datastorage-les-emplois

Pour run_management_command.sh c'est bien tenté mais tu pourras pas trop empêcher la duplication de code, donc dans le cas actuel c'est presque préférable d'accepter cette duplication que de créer un mille-feuille de script s’appelant.

Merci pour l'astuce ! J'ai testé la commande et tout va bien

rsebille · 2024-12-16T17:53:35Z

clevercloud/cron.json

@@ -31,6 +31,7 @@
  "0 0 * * 1 $ROOT/clevercloud/run_management_command.sh shorten_active_sessions",
  "0 2 * * 1 $ROOT/clevercloud/crons/populate_metabase_matomo.sh",
  "0 12 * * 1 $ROOT/clevercloud/run_management_command.sh import_ea_eatt --from-asp --wet-run",
+  "0 12 * * 1 $ROOT/clevercloud/upload_to_pilotage.sh",


Je pense que midi c'est bien, on changera si on rencontre des problèmes.
A noter que l'heure système des machines Clever est en UTC et que l'ASP est en heure de métropole (CET/CEST suivant la saison) donc on devrait être bon, et avoir en plus de la marge :).

... Dec 2 11:14 fluxIAE_ITOU_20241202_115450609.tar.gz ... Dec 9 09:48 fluxIAE_ITOU_20241209_102749998.tar.gz ... Dec 16 09:51 fluxIAE_ITOU_20241216_103042036.tar.gz

clevercloud/upload_to_pilotage.sh

config/settings/test.py

rsebille · 2024-12-16T18:08:01Z

itou/metabase/management/commands/upload_data_to_pilotage.py

+        filename = os.path.basename(filename)
+        filepath = os.path.join("asp_riae_shared_bucket", filename)


Sauf erreur de ma part, ces 2 lignes ne servent à rien, tu ne fait que recréer ce qui t'es passé en paramètre donc filename

Suggested change

filename = os.path.basename(filename)

filepath = os.path.join("asp_riae_shared_bucket", filename)

Mais si tu veux garder filename alors faut changer le nom du paramètre du script en filepath vu que c'est ce qui est passé en argument :).

Suggested change

filename = os.path.basename(filename)

filepath = os.path.join("asp_riae_shared_bucket", filename)

filename = os.path.basename(filepath)

Je prends le deuxième. J'avais l'intention de forcer l'utilisation du asp_riae_shared_bucket, mais la deuxième solution marche pour moi 👍

itou/metabase/management/commands/upload_data_to_pilotage.py

rsebille · 2024-12-16T18:28:29Z

clevercloud/upload_to_pilotage.sh

+    exit 0
+fi
+
+/bin/bash $ROOT/clevercloud/run_management_command.sh upload_data_to_pilotage "$FLUX_IAE_FILE"


Je n'ai pas testé sur prod. On peut faire un dry-run sur une recette jetable ?

Oui tu pourras tester sur une recette jetable, ceci devrais fonctionner :

Tu te connectes en SSH sur ta recette jetable

Puis mkdir "$APP_HOME/asp_riae_shared_bucket && touch "$APP_HOME/asp_riae_shared_bucket/fluxIAE_ITOU_$(date '+%Y%m%d_%H%M%S').tar.gz" pour créer un dummy file

Tu lances ton script et vérifie que le fichier est bien reçu dans le bucket S3 pilotage-datastorage-les-emplois

Pour run_management_command.sh c'est bien tenté mais tu pourras pas trop empêcher la duplication de code, donc dans le cas actuel c'est presque préférable d'accepter cette duplication que de créer un mille-feuille de script s’appelant.

github-actions · 2024-12-19T11:42:09Z

🥁 La recette jetable est prête ! 👉 Je veux tester cette PR !

rsebille · 2024-12-23T11:47:23Z

Pour info, suite à mes tests je viens de push des changements afin de :

Se passer de clevercloud/upload_to_pilotage.sh qui n'apporte rien sauf à faire une indirection
On synchronise tout les fichiers présents et pas juste le dernier, en partie lié au point précédent mais aussi parce que c'est plus simple a expliquer et que ça peux nous éviter des problèmes de ne pas jouer avec le -mtime
Modification pour l'affichage de la progression : Ajout d'un lock, affichage seulement tout les 5%, et utilisation de filesizeformat()
Suite au changement de logique de synchronisation on s'autorise à lancer la tâche plusieurs fois le lundi matin afin d'être plus robuste, que ce soit en cas de MEP des emplois ou d'un retard dans l'arrivée du fichier.
Ajout de tests, obligatoire pour une tâche cron

Use a cronjob to share imported data with the pilotage via an S3 bucket.

calummackervoy · 2025-01-07T10:20:31Z

itou/metabase/management/commands/upload_data_to_pilotage.py

+        parser.add_argument("--wet-run", dest="wet_run", action="store_true")
+
+    def _upload_file(self, file: pathlib.Path, *, wet_run=False):
+        lock = threading.Lock()


👍 🔒
Si j'ai bien compris ce lock protège contre le cas où log_progress est plus lente que le téléversement des fichiers

C'est pas tout à fait ça, c'est bien lié à une histoire de temps mais c'est plus de la synchronicité qu'une durée, même si tu as raison que plus log_progress est rapide (et aussi une histoire de comment elle est codé) moins le problème est visible.
L'appel de la callback de .upload_file() est asynchrone, sûrement pour ne pas bloquer le téléversement inutilement, mais donc on peux se retrouver avec de multiples appels en parallèle et vu qu'on a des variables non-locale on doit forcer la synchronicité lors de leur modification sinon leur état peux changer entre chaque ligne.
D'ailleurs dans l'exemple de la documentation le lock est mis par défaut : https://boto3.amazonaws.com/v1/documentation/api/latest/guide/s3-uploading-files.html#the-callback-parameter

Le variable lock ici est locale au scope du fonction _upload_file, on n'a pas besoin que ce soit self.lock et définit dans __init__ comme dans l'example ?

On pourrais mais pas besoin non, et vu que la variable n'est utilisée que dans ._upload_file() autant la garder au plus proche de ses utilisations.
On aurais eu besoin d'une variable d'instance si log_progress() avait été une méthode de classe, mais vu que là c'est une inner function c'est pas nécessaire ;).

OK, je vais le merger :) merci pour les explications !

calummackervoy · 2025-01-07T10:21:08Z

itou/metabase/management/commands/upload_data_to_pilotage.py

+                progress = int((bytes_transferred / file_size) * 100)
+                if progress > previous_progress and progress % 5 == 0:
+                    self.stdout.write(
+                        f"> {file.name}: {filesizeformat(bytes_transferred)}/{filesizeformat(file_size)} transferred ({progress}%)."  # noqa: E501


filesizeformat est un nouveau filtre pour moi 👍

calummackervoy

Merci pour tes améliorations ! Quand prévoit-on le merger ? :)

rsebille

Merci pour tes améliorations ! Quand prévoit-on le merger ? :)

Quand tu veux :), je ne voulais pas fusionner sans que tu repasses sur mes modifications dans le cas où j'aurais raté un truc.

rsebille · 2025-01-07T13:14:36Z

itou/metabase/management/commands/upload_data_to_pilotage.py

+        parser.add_argument("--wet-run", dest="wet_run", action="store_true")
+
+    def _upload_file(self, file: pathlib.Path, *, wet_run=False):
+        lock = threading.Lock()


C'est pas tout à fait ça, c'est bien lié à une histoire de temps mais c'est plus de la synchronicité qu'une durée, même si tu as raison que plus log_progress est rapide (et aussi une histoire de comment elle est codé) moins le problème est visible.
L'appel de la callback de .upload_file() est asynchrone, sûrement pour ne pas bloquer le téléversement inutilement, mais donc on peux se retrouver avec de multiples appels en parallèle et vu qu'on a des variables non-locale on doit forcer la synchronicité lors de leur modification sinon leur état peux changer entre chaque ligne.
D'ailleurs dans l'exemple de la documentation le lock est mis par défaut : https://boto3.amazonaws.com/v1/documentation/api/latest/guide/s3-uploading-files.html#the-callback-parameter

calummackervoy added the tech label Dec 11, 2024

calummackervoy requested a review from rsebille December 11, 2024 16:01

calummackervoy self-assigned this Dec 11, 2024

calummackervoy force-pushed the calum/upload-flux-iae-to-s3 branch from 7f25046 to dbf7051 Compare December 11, 2024 16:04

rsebille reviewed Dec 12, 2024

View reviewed changes

calummackervoy mentioned this pull request Dec 12, 2024

Pilotage: Supprimer populate_metabase_flux_iae #5260

Draft

1 task

calummackervoy force-pushed the calum/upload-flux-iae-to-s3 branch from dbf7051 to 167c62e Compare December 12, 2024 17:52

calummackervoy commented Dec 12, 2024

View reviewed changes

calummackervoy force-pushed the calum/upload-flux-iae-to-s3 branch 3 times, most recently from 513ba00 to 95574d0 Compare December 16, 2024 14:38

rsebille reviewed Dec 16, 2024

View reviewed changes

calummackervoy force-pushed the calum/upload-flux-iae-to-s3 branch from 95574d0 to 187f561 Compare December 19, 2024 11:36

calummackervoy added the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Dec 19, 2024

calummackervoy force-pushed the calum/upload-flux-iae-to-s3 branch from 187f561 to 48cef10 Compare December 19, 2024 14:11

calummackervoy removed the 1-recette-jetable [Payé à l’heure] Crée une recette jetable sur CC label Dec 19, 2024

rsebille force-pushed the calum/upload-flux-iae-to-s3 branch from 48cef10 to 58c5edf Compare December 23, 2024 11:38

metabase: Add upload_data_to_pilotage command and cron schedule

9128832

Use a cronjob to share imported data with the pilotage via an S3 bucket.

rsebille force-pushed the calum/upload-flux-iae-to-s3 branch from 58c5edf to 9128832 Compare December 23, 2024 17:42

calummackervoy commented Jan 7, 2025

View reviewed changes

rsebille approved these changes Jan 7, 2025

View reviewed changes

calummackervoy added this pull request to the merge queue Jan 7, 2025

Merged via the queue into master with commit f3effdd Jan 7, 2025
11 checks passed

calummackervoy deleted the calum/upload-flux-iae-to-s3 branch January 7, 2025 15:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pilotage: Partage du fichier import fluxIAE via nouveau bucket S3 #5249

Pilotage: Partage du fichier import fluxIAE via nouveau bucket S3 #5249

calummackervoy commented Dec 11, 2024

rsebille left a comment

calummackervoy Dec 12, 2024

rsebille Dec 16, 2024

calummackervoy Dec 12, 2024 •

edited

Loading

calummackervoy Dec 12, 2024

rsebille Dec 16, 2024

calummackervoy Dec 19, 2024

rsebille Dec 16, 2024

rsebille Dec 16, 2024

calummackervoy Dec 19, 2024

rsebille Dec 16, 2024

github-actions bot commented Dec 19, 2024

rsebille commented Dec 23, 2024 •

edited

Loading

calummackervoy Jan 7, 2025

rsebille Jan 7, 2025

calummackervoy Jan 7, 2025

rsebille Jan 7, 2025

calummackervoy Jan 7, 2025

calummackervoy Jan 7, 2025 •

edited

Loading

calummackervoy left a comment

rsebille left a comment

rsebille Jan 7, 2025

		filename = os.path.basename(filename)
		filepath = os.path.join("asp_riae_shared_bucket", filename)

	filename = os.path.basename(filename)
	filepath = os.path.join("asp_riae_shared_bucket", filename)
	filename = os.path.basename(filepath)

Pilotage: Partage du fichier import fluxIAE via nouveau bucket S3 #5249

Pilotage: Partage du fichier import fluxIAE via nouveau bucket S3 #5249

Conversation

calummackervoy commented Dec 11, 2024

🤔 Pourquoi ?

🚨 À vérifier

rsebille left a comment

Choose a reason for hiding this comment

Étape 1 - PR emplois : Téléversement du flux IAE

Étape 2 - PR pilotage : DAG flux IAE

Étape 3 - PR emplois : Nettoyage de populate_metabase_flux_iae

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calummackervoy Dec 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Dec 19, 2024

rsebille commented Dec 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calummackervoy Jan 7, 2025 • edited Loading

Choose a reason for hiding this comment

calummackervoy left a comment

Choose a reason for hiding this comment

rsebille left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Étape 3 - PR emplois : Nettoyage de `populate_metabase_flux_iae`

calummackervoy Dec 12, 2024 •

edited

Loading

rsebille commented Dec 23, 2024 •

edited

Loading

calummackervoy Jan 7, 2025 •

edited

Loading