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

Candidatures : Utiliser un autre domaine pour le lien vers le CV [GEN-1628] #4053

Merged
merged 1 commit into from
May 9, 2024

Conversation

francoisfreitag
Copy link
Contributor

🤔 Pourquoi ?

Edge affiche une alerte de sécurité lorsque l’utilisateur est redirigé vers https://cellar-c2.services.clever-cloud.com, qui rend l’accès aux CVs difficile.

🍰 Comment ?

Clever a activé un contournement temporaire via un domaine alternatif : par.cellar.clever-cloud.com.

@francoisfreitag francoisfreitag self-assigned this May 9, 2024
@francoisfreitag francoisfreitag changed the title Candidatures : Utiliser un autre domaine pour le lien vers le CV Candidatures : Utiliser un autre domaine pour le lien vers le CV [GEN-1628] May 9, 2024
Copy link

@francoisfreitag
Copy link
Contributor Author

Pour info, la migration met environ 5 minutes à passer en local.



def forwards(apps, editor):
print()
Copy link
Collaborator

Choose a reason for hiding this comment

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

👀

Copy link
Contributor Author

@francoisfreitag francoisfreitag May 9, 2024

Choose a reason for hiding this comment

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

C’est exprès, c’est pour revenir à la ligne avant d’écrire.
Django a son moyen d’afficher les migations :

Apply migrations
  0003_workaround_clever_dangerous_domain_ms... 

Sans \n après les points de suspension.
Sans le print() :

  0003_workaround_clever_dangerous_domain_ms... Updated 20000 job applications, migration duration XXXs
Updated 40000 job applications, migration duration XXXs
OK

Avec le print() :

  0003_workaround_clever_dangerous_domain_ms... 
Updated 20000 job applications, migration duration XXXs
Updated 40000 job applications, migration duration XXXs
OK

Copy link
Collaborator

Choose a reason for hiding this comment

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

Aaaah ! Merci de l'explication. :)

Copy link
Contributor

@rsebille rsebille left a comment

Choose a reason for hiding this comment

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

Ça serais pas "mieux" de le faire au moment de l'affichage ? Comme ça on gère les anciens et les nouveaux ? Et on se tape pas une migration à chaque fois qu'on veux changer le domaine ?
Surtout que resume_link n'est affiché qu'à un seul endroit actuellement.
Ça pourrais même être un templatefilter pour pouvoir le réutiliser partout, car le problème va aussi être présent pour les endroits qui utilise un File() (CàP et bilan AI IIRC).

@francoisfreitag
Copy link
Contributor Author

Ça serais pas "mieux" de le faire au moment de l'affichage ? Comme ça on gère les anciens et les nouveaux ? Et on se tape pas une migration à chaque fois qu'on veux changer le domaine ? Surtout que resume_link n'est affiché qu'à un seul endroit actuellement. Ça pourrais même être un templatefilter pour pouvoir le réutiliser partout, car le problème va aussi être présent pour les endroits qui utilise un File() (CàP et bilan AI IIRC).

Nope. Pour le CaP et les AI, on génère un lien présigné avec django-storages, qui utilise CELLAR_ADDON_HOST.

Il reste donc resume_link. Je trouve plus cohérent d’utiliser ce qui est en DB, et j’espère bien ne pas avoir souvent à changer ces données. Avec un peu de chance (et beaucoup d’optimisme et pas mal de boulot), on pourra migrer vers cdn.emplois.inclusion.beta.gouv.fr la prochaine fois. Dans un futur proche, refaire l’opération inverse devrait être un copier coller de cette migration sans difficulté.
Après, comme la redirection est effectivement temporaire, on pourrait jouer à trouver tous les resume_link et les passer dans la moulinette. Mais avec CELLAR_ADDON_HOST surchargé dans Clever, les CV envoyés depuis ce matin sont déjà en train d’être enregistrés avec le domaine temporaire par.cellar.clever-cloud.com. Plutôt que de devoir s’adapter à ce qu’on a en DB systématiquement, je trouve plus facile de garder une donnée cohérente en DB.

Copy link
Contributor

@rsebille rsebille left a comment

Choose a reason for hiding this comment

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

Effectivement, je l'avais en tête mais j'avais pas connecté avec le changement de CELLAR_ADDON_HOST 😀.

Pour la durée de la migration je sais plus trop le timeout coté Clever donc peut-être la marquer atomic = False pour ne pas perdre l'avancement au cas ou ?

Edge shows a security warning when directed to
https://cellar-c2.services.clever-cloud.com. Clever temporary works
around the issue by providing an alternate domain:
par.cellar.clever-cloud.com.

Avoid security warnings for existing resumes.
@francoisfreitag
Copy link
Contributor Author

atomic = False était carrément mon intention, merci !

@francoisfreitag francoisfreitag added this pull request to the merge queue May 9, 2024
Merged via the queue into master with commit 699d611 May 9, 2024
11 checks passed
@francoisfreitag francoisfreitag deleted the ff/cv-mig branch May 9, 2024 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants