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

Create upgrade_to_0.8.0.sql #100

Merged
merged 4 commits into from
Jul 26, 2021
Merged

Create upgrade_to_0.8.0.sql #100

merged 4 commits into from
Jul 26, 2021

Conversation

Jersig
Copy link
Contributor

@Jersig Jersig commented May 10, 2021

Update of the latest production version of the "adresse" schema.
It implements various objects type :
• Functions
• tables, sequences, default values
• views
• indexes
• triggers
• constrains
• comments
This upgrade aims to create new tables and setup several functions which automatically update different fields in our tables. It allow to produce assessment view used to export data with a structure expected by the project partners.

  • Ticket : #

@Gustry
Copy link
Member

Gustry commented May 11, 2021

Merci pour la contribution @Jersig
Est-ce qu'il y a une raison pour vouloir faire 2 versions de l'extension 0.8.0 ici #100 et aussi une 0.9.0 dans #101 ?
Pourquoi pas dans la même release ?

Pour faire du SQL, il y a 2 choses :

Il faut que les 2 soit synchro.

Je viens de d'approuver le lancement des tests et avec surprise, le test est vert ... Je pense qu'il y a un soucis de notre côté suite à la migration Travis vers GitHub récemment. Parce que ton PR n'ayant que un fichier de migration, il y a donc une différence si quelqu'un fait l'installation d'une nouvelle base. Je dois vérifier ca.

@Gustry
Copy link
Member

Gustry commented May 11, 2021

@Jersig Je me suis permis de faire un "rebase", CAD mettre à jour ton PR, suite à la découverte d'un problème possible ce matin.

Donc comme dit ci-dessus, le test des migrations plante désormais car le PR ne couvre pas l'installation d'une base.

Merci pour la puce à l'oreille :)

@pdrillin pdrillin force-pushed the patch-8 branch 17 times, most recently from 9a43214 to e50980e Compare July 8, 2021 14:40
@pdrillin pdrillin requested a review from rldhont July 16, 2021 07:35
gestion_base_adresse/test/test_load_structure.py Outdated Show resolved Hide resolved
gestion_base_adresse/test/test_load_structure.py Outdated Show resolved Hide resolved
gestion_base_adresse/test/test_load_structure.py Outdated Show resolved Hide resolved
gestion_base_adresse/install/sql/adresse/70_COMMENT.sql Outdated Show resolved Hide resolved
gestion_base_adresse/install/sql/adresse/70_COMMENT.sql Outdated Show resolved Hide resolved
gestion_base_adresse/install/sql/adresse/60_CONSTRAINT.sql Outdated Show resolved Hide resolved
gestion_base_adresse/install/sql/adresse/50_TRIGGER.sql Outdated Show resolved Hide resolved
gestion_base_adresse/install/sql/adresse/40_INDEX.sql Outdated Show resolved Hide resolved
gestion_base_adresse/install/sql/adresse/40_INDEX.sql Outdated Show resolved Hide resolved
@pdrillin pdrillin force-pushed the patch-8 branch 2 times, most recently from b5cfa0d to 73752df Compare July 19, 2021 11:29
@@ -37,6 +37,7 @@ def tearDown(self) -> None:
del self.connection
time.sleep(1)

@unittest.skip("Test desable because timeout")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
@unittest.skip("Test desable because timeout")
@unittest.skip("Test disabled because of timeout")

Can you elaborate the problem ? Does it work on on your local machine ? Is the problem only on GH Action ? (if yes, skip only on GH Action, not always, with environnement variable)
If you remove the latest migration, does it work ?
This test is IMHO important, so it would be good to now why there is a timeout and where in the code.

Copy link
Contributor

Choose a reason for hiding this comment

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

On my local machine I ran it for about an hour and a half without it being finished

Copy link
Member

Choose a reason for hiding this comment

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

More than 2 minutes, there is already something wrong, it's only a SQL script.
But is-it the latest migration making this trouble ?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, is the migration 0.8.0

@rldhont
Copy link
Contributor

rldhont commented Jul 23, 2021

Ajouter BEGIN et COMMIT aux fichiers SQL

Jersig and others added 2 commits July 23, 2021 13:56
Update of the latest production version of the "adresse" schema.
It implements various objects type :
•	Functions
•	tables, sequences, default values
•	views
•	indexes
•	triggers
•	constrains
•	comments
This upgrade aims to create new tables and setup several functions which automatically update different fields in our tables. It allow to produce assessment view used to export data with a structure expected by the project partners.
@pdrillin
Copy link
Contributor

@Gustry @rldhont j'ai trouvé, vous pouvez review

@rldhont
Copy link
Contributor

rldhont commented Jul 24, 2021

LGTM

Copy link
Member

@Gustry Gustry left a comment

Choose a reason for hiding this comment

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

LGTM.

Good idea the BEGIN COMMIT in the install file, thanks

Can you do it in all migration files with the test checking these keywords (begin and commit) ?
A separate PR is ok. Check in veloroutes for the test.

@pdrillin pdrillin merged commit a7dae29 into 3liz:master Jul 26, 2021
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.

4 participants