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

Dataset : ajout de colonnes pour logos personnalisés #3741

Merged
merged 4 commits into from
Jan 24, 2024

Conversation

AntoineAugusti
Copy link
Member

Première étape pour #3701

Cette PR :

  • ajoute 2 nouvelles colonnes dans dataset : custom_logo et custom_full_logo qui contiennent des URLs vers des logos personnalisés. Leurs noms suivent les colonnes logo et full_logo
  • adapte le code pour utiliser le logo provenant de data.gouv.fr ou le logo personnalisé si défini
  • les tests correspondants
  • un script permettant de générer les logos aux tailles attendues (max 500px pour un logo normal et un thumbnail de 100x100 pixels), indiquant les commandes à effectuer pour upload les logos vers le nouveau bucket créé pour l'occasion et la commande SQL à exécuter

À suivre : adaptation de l'Espace Producteur pour offrir la possibilité aux producteurs d'envoyer un logo personnalisé, qui préviendra notre équipe pour effectuer l'opération de préparation et upload des images. On automatisera cette étape plus tard si on a beaucoup de demandes, on essaie d'éviter de déployer vix sur notre infrastructure en production.

@@ -48,6 +48,11 @@ defmodule DB.Dataset do
field(:latest_data_gouv_comment_timestamp, :utc_datetime)
field(:archived_at, :utc_datetime_usec)
field(:custom_tags, {:array, :string}, default: [])
# URLs for custom logos.
# Currently we host custom logos in Cellar buckets.
# https://transport-data-gouv-fr-logos-(logos|staging|dev).cellar-c2.services.clever-cloud.com/
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Les buckets ont déjà été créés sur Clever Cloud

|> Enum.map_join("\n", fn path ->
"s3cmd put --acl-public --add-header='Cache-Control: public, max-age=604800' #{path} s3://transport-data-gouv-fr-logos-prod/"
end)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un exemple de logo :

Les headers sont bons et permettent d'avoir une réponse 304 quand nécessaire (etag + cache-control). Ceci permettra de diminuer l'usage de la bande passante.

$ curl -I https://transport-data-gouv-fr-logos-prod.cellar-c2.services.clever-cloud.com/foobar_full.jpeg
HTTP/1.1 200 OK
content-length: 18994
accept-ranges: bytes
last-modified: Tue, 23 Jan 2024 14:36:36 GMT
x-rgw-object-type: Normal
ETag: "ebe37a8b37ddf26fc1906f0846616092"
x-amz-meta-s3cmd-attrs: atime:1706020049/ctime:1706020591/gid:0/gname:wheel/md5:ebe37a8b37ddf26fc1906f0846616092/mode:33188/mtime:1706020591/uid:502/uname:antoineaugusti
cache-control: public, max-age=604800
x-amz-storage-class: STANDARD
x-amz-request-id: tx00000000000000ad6e34e-0065b0d1a8-2584b297-default
content-type: image/jpeg
date: Wed, 24 Jan 2024 09:00:24 GMT

@AntoineAugusti AntoineAugusti marked this pull request as ready for review January 24, 2024 09:05
@AntoineAugusti AntoineAugusti requested a review from a team as a code owner January 24, 2024 09:05
Copy link
Contributor

@thbar thbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

L'approche discutée au point dév de ne pas aller coller tout de suite les librairies de traitement d'image en production me paraît toujours la bonne et je trouve ça cool qu'on puisse s'appuyer sur Mix.install/2 pour ça, c'est encore un bon exemple d'usage de cette possibilité...

J'ai mis quelques retours et suggestions, pour DRYer la configuration idéalement.

Mais ça paraît bien et ça traite le sujet de façon assez low-tech-smooth 😄, ça nous fait temporiser sur une "grosse" dépendance et des impacts possibles, donc c'est cool !

@@ -0,0 +1,39 @@
# Run script:
# `elixir scripts/custom_logo.exs /tmp/logo.jpg f0700f5f9954`
Mix.install([{:image, "~> 0.37"}])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ref:

An approachable image processing library primarily based upon Vix and libvips that is NIF-based, fast, multi-threaded, pipelined and has a low memory footprint.

@AntoineAugusti
Copy link
Member Author

@thbar Merci pour la review ! J'ai extrait les URLs des buckets dans la configuration et adapté pour les différents environnements.

Je garde le nom de colonne custom_* car ça me parait être explicite et pour évidemment de faire plus de changements 😄

Copy link
Contributor

@thbar thbar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merci @AntoineAugusti ça paraît tout bon !

@AntoineAugusti AntoineAugusti added this pull request to the merge queue Jan 24, 2024
Merged via the queue into master with commit 8c9bb84 Jan 24, 2024
4 checks passed
@AntoineAugusti AntoineAugusti deleted the dataset_add_custom_logos branch January 24, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants