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

Mises à jour techniques relatives à Ecto #4347

Merged
merged 10 commits into from
Dec 10, 2024
Merged

Mises à jour techniques relatives à Ecto #4347

merged 10 commits into from
Dec 10, 2024

Conversation

thbar
Copy link
Contributor

@thbar thbar commented Nov 29, 2024

Toujours extrait de ce que je fais sur:

J'isole ici 7 mises à jour, qui permettent par ricochet de supprimer une dépendance à la librairie connection.

postgrex dans sa version 0.16 (https://hex.pm/packages/postgrex/0.16.5) qu'on utilisait jusque là, dépendant de connection (https://hex.pm/packages/connection), qui n'est en fait plus souhaitable:

Note: with the inclusion of gen_statem in Erlang/OTP, this project is no longer necessary.
We may release new versions if necessary to keep compatibility with Erlang/OTP and Elixir but otherwise this package is no longer recommended for new projects.

Pour pouvoir upgrader, il faut débloquer les dépendances "interconnectées" sur cette contrainte, ce que je fais ici.

Points de vigilance

⚠️ La mise à jour de Ecto (3.11 -> 3.12) inclut de nombreuses améliorations, et un point de vigilance dans le changelog Potential incompatibilities. Il faut donc suivre ce qui se passe au niveau tests et déploiement.

L'écart de version Postgrex est important (0.16 -> 0.19). Il y a des changements du type Respect precision for interval, time, timestamp, and timestamptz.

TODOs

CleanShot 2024-11-29 at 09 25 45@2x

Changelogs

(initially generated with elixir generate_deps_changelogs.exs)

@thbar thbar added the dette technique Entretien & maintenance générale, nécessaire pour que le code reste de bonne qualité label Nov 29, 2024
@thbar thbar self-assigned this Nov 29, 2024
@thbar thbar marked this pull request as ready for review November 29, 2024 08:27
@thbar thbar requested a review from a team as a code owner November 29, 2024 08:27
@thbar thbar removed their assignment Nov 29, 2024
@thbar
Copy link
Contributor Author

thbar commented Nov 29, 2024

Le CI est bloqué, mais pas en lien avec les tests.

@AntoineAugusti
Copy link
Member

@thbar La dernière exécution semble montrer un problème dans les migrations, peut-être en lien avec cette PR et la màj d'Ecto ?

@thbar
Copy link
Contributor Author

thbar commented Dec 2, 2024

Erreur reproduite en local:

$ mix ecto.drop
$ mix ecto.create
$ mix ecto.migrate

18:17:13.948 [info] == Running 20221206135302 DB.Repo.Migrations.DataImportDelete.up/0 forward
** (KeyError) key :from not found in: %Ecto.Migration.Reference{
  name: nil,
  prefix: nil,
  table: "data_import",
  column: :id,
  type: :bigserial,
  on_delete: :delete_all,
  on_update: :nothing,
  validate: true,
  with: [],
  match: nil,
  options: []
}

Il va falloir retoucher les migrations pour que ça passe.

Je me renseigne côté changelog / GitHub.

@AntoineAugusti AntoineAugusti linked an issue Dec 9, 2024 that may be closed by this pull request
@AntoineAugusti
Copy link
Member

J'ai réparé la migration qui posait problème et vérifié que la table obtenue était bien la même en test et en production. Voir #4358

Le from: était une erreur de syntaxe, je ne comprends pas pourquoi en revanche il y avait une erreur et la nécessité de supprimer une clé étrangère.

Screenshot 2024-12-09 at 10 15 53
image

@thbar
Copy link
Contributor Author

thbar commented Dec 9, 2024

J'ai réparé la migration qui posait problème et vérifié que la table obtenue était bien la même en test et en production. Voir #4358

Top, merci !

Le from: était une erreur de syntaxe, je ne comprends pas pourquoi en revanche il y avait une erreur et la nécessité de supprimer une clé étrangère.

Yes, ça je pense qu'on le découvrira si on finit par #3596 squasher les migrations à un moment.

Nickel, il ne reste plus qu'à:

@thbar thbar self-assigned this Dec 9, 2024
@thbar
Copy link
Contributor Author

thbar commented Dec 9, 2024

Je pousse le tout sur prochainement.

@thbar
Copy link
Contributor Author

thbar commented Dec 9, 2024

Je ne vois pas d'erreur massive sur prochainement, je mergerai demain matin.

repro.exs Outdated Show resolved Hide resolved
@thbar
Copy link
Contributor Author

thbar commented Dec 10, 2024

Je n'ai pas vu de souci sur prochainement, je vais déployer.

@thbar thbar enabled auto-merge December 10, 2024 08:02
@thbar
Copy link
Contributor Author

thbar commented Dec 10, 2024

@etalab/transport-tech est-ce que quelqu'un peut reviewer pour envoi en production en mode "attended" ? Merci !

Copy link
Member

@AntoineAugusti AntoineAugusti left a comment

Choose a reason for hiding this comment

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

Un fichier à déplacer et ensuite 🚢

repro.exs Outdated Show resolved Hide resolved
@thbar thbar added this pull request to the merge queue Dec 10, 2024
@AntoineAugusti AntoineAugusti removed this pull request from the merge queue due to a manual request Dec 10, 2024
github-merge-queue bot pushed a commit that referenced this pull request Dec 10, 2024
* mix deps.update ecto_interval ecto_sql ecto postgrex ecto_psql_extras

* Add reproduction for the regression

* Try to fix migration

---------

Co-authored-by: Antoine Augusti <[email protected]>
@thbar thbar enabled auto-merge December 10, 2024 08:32
@thbar thbar added this pull request to the merge queue Dec 10, 2024
@thbar thbar removed this pull request from the merge queue due to a manual request Dec 10, 2024
@thbar thbar added this pull request to the merge queue Dec 10, 2024
Merged via the queue into master with commit 241ac29 Dec 10, 2024
4 checks passed
@thbar thbar deleted the deps-upgrades-part-1 branch December 10, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dette technique Entretien & maintenance générale, nécessaire pour que le code reste de bonne qualité
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mise à jour Ecto impossible - souci lié aux migrations
2 participants