search

ARUP-CAS / aiscr-webamcr

Archaeological Map of the Czech Republic (AMCR)
https://amcr-info.aiscr.cz/
GNU General Public License v3.0
5 stars 0 forks source link

Automatizace migrací a tvorby DB z Django models #841

Closed motyc closed 9 months ago

motyc commented 1 year ago

Přehled migračních souborů, jejich popis a stav (vč. zdůvodnění odstranění některých souborů) je uveden zde: https://docs.google.com/spreadsheets/d/1szarV2dwbS40OYMmhMfjGhtR3NkNYioX/edit#gid=1817932761

Zohlednit opravy v aplikaci

Drobné ladění

Postup

Je třeba vyjasnit/vyřešit (mimo rámec tohoto issue, avšak souvisí)

pesikj commented 1 year ago

@motyc

@pesikj Navrhoval bych odstranit některé zastaralé verze DB. které nám jen visí na serveru, ale k ničemu patrně nejsou potřeba, jde o:

Souhlasím.

Ostatní body si musím projít, to jsou věci, které dělal ještě Juraj.

motyc commented 1 year ago

@pesikj Prošel jsem objevené problémy v migracích podle dohody a připravil #895, což by mělo řadu věcí opravovat. Také jsem aktualizoval první post v tomto vlákně a odstranil zastaralé informace. Upozornil bych zejména na nově zjištěný problém s encrypt_passwords.py - tam bude třeba Váš zásah.

Nyní zkusím udělat to slíbené srovnání DB.

motyc commented 1 year ago

@pesikj Tak porovnání je hotovo. Nejasností či chyb bylo minimum. Všechny potřebné opravy jsem ještě zahrnul do #895. Po merge a vyřešení toho encrypt_passwords.py by asi bylo dobré provést ještě jeden test celé migrace. Jinak ty modely lze už s klidným svědomím upravit a vygenerovat prázdnou DB. Pak udělám nové porovnání.

motyc commented 1 year ago

@pesikj Na list https://docs.google.com/spreadsheets/d/1szarV2dwbS40OYMmhMfjGhtR3NkNYioX/edit#gid=1710645624 jsem vložil návrh pořadí migrace dat mezi naší DB a DB vygenerovanou z modelů. V tomto pořadí by neměl vzniknout problém.

pesikj commented 1 year ago

@motyc Tato definice tabulky není správně, protože archeologicky_zaznam_historie_fkey má ON DELETE SET NULL a současně historie má nastaveno NOT NULL. Obávám se, že takových chyb je tam více, protože PostgreSQL to nehlídá, Django ano. Má být použíto NO ACTION nebo něco jiného?

CREATE TABLE IF NOT EXISTS public.archeologicky_zaznam
(
    id integer NOT NULL DEFAULT nextval('archeologicky_zaznam_id_seq'::regclass),
    typ_zaznamu text COLLATE pg_catalog."default" NOT NULL,
    pristupnost integer NOT NULL DEFAULT 857,
    ident_cely text COLLATE pg_catalog."default" NOT NULL,
    historie integer NOT NULL,
    uzivatelske_oznaceni text COLLATE pg_catalog."default",
    stav smallint NOT NULL,
    hlavni_katastr integer,
    CONSTRAINT archeologicky_zaznam_pkey PRIMARY KEY (id),
    CONSTRAINT archeologicky_zaznam_historie_key UNIQUE (historie),
    CONSTRAINT archeologicky_zaznam_ident_cely_key UNIQUE (ident_cely),
    CONSTRAINT archeologicky_zaznam_historie_fkey FOREIGN KEY (historie)
        REFERENCES public.historie_vazby (id) MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE SET NULL,
    CONSTRAINT archeologicky_zaznam_hlavni_katastr_fkey FOREIGN KEY (hlavni_katastr)
        REFERENCES public.ruian_katastr (id) MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE NO ACTION,
    CONSTRAINT archeologicky_zaznam_pristupnost_fkey FOREIGN KEY (pristupnost)
        REFERENCES public.heslar (id) MATCH SIMPLE
        ON UPDATE CASCADE
        ON DELETE NO ACTION,
    CONSTRAINT check_correct_type CHECK (typ_zaznamu = 'L'::text OR typ_zaznamu = 'A'::text)
)
motyc commented 1 year ago

@pesikj Pravda. Ono to je situace, co patrně v praxi nikdy nenastane, ale můžete to opravit na NO ACTION (platí i pro další obdobné případy).

pesikj commented 1 year ago

@motyc Tento řádek

ALTER TABLE vyskovy_bod ALTER COLUMN geom TYPE geometry(POINTZ, 5514) USING ST_SetSRID(geom, 5514);

generuje chybu 

Column has Z dimension but geometry does not

U databáze test_migrace_20230123_all je vše v pořádku, je to zřejmě tedy další chyba dat vložených aplikací.

motyc commented 1 year ago

@pesikj Ano, zrovna u VB jsou ta data hodně rozbitá, jak se s tím hodně laborovalo tam a zpět...

motyc commented 1 year ago

@pesikj Je trošku nepříjemné, že jste na DB test_migrace_20230123_all aplikoval od posledně nějaké změny, i když jsme ji měli jako zálohu, ale to se nedá nic dělat (nejde teď poznat, zda není chybně něco v obou DB zároveň, protože nemáme autoritní zkontrolovanou verzi). Zde je každopádně pár divných rozdílů mezi DB test_migrace_20230123_alla test_migrace_20230207.

Je mi tedy trošku divné, že by ta migrace do test_migrace_20230207 pomocí https://github.com/ARUP-CAS/aiscr-webamcr/blob/dev/migrace/migration_batch.sh skutečně proběhla bez chyb.

Výsledky srovnání test_migrace_20230207 s test_migrace_20230123_all ještě musím analyzovat.

pesikj commented 1 year ago

@motyc Závěry z přenosu dat do nové databáze:

pesikj commented 1 year ago

@motyc

Je trošku nepříjemné, že jste na DB test_migrace_20230123_all aplikoval od posledně nějaké změny

To se omlouvám, na to jsem zapomněl, ale těch změn by bylo minimum. Potřeboval jsem otestovat změnu geometrie, jinak si změn nejsem vědom.

Je mi tedy trošku divné, že by ta migrace do test_migrace_20230207 pomocí https://github.com/ARUP-CAS/aiscr-webamcr/blob/dev/migrace/migration_batch.sh skutečně proběhla bez chyb.

Určitě neproběhla bez chyb, nějaké chyby se objevily. Pošlu vám log.

motyc commented 1 year ago

@pesikj V pořádku, není to zásadní, jen se to hůř sleduje... Log bude fajn. Musíme zjistit, proč se to neaplikovalo. Teď bychom to měli udržovat "čisté", tj. jakmile se při automatické migraci objeví chyba, hned bychom ji měli zaznamenat do https://docs.google.com/spreadsheets/d/1szarV2dwbS40OYMmhMfjGhtR3NkNYioX/edit#gid=1817932761 a řešit, aby se nestalo, že něco opomeneme a dožene nás to jednou při ostré migraci.

motyc commented 1 year ago

@pesikj

  • Pole typ_zmeny u modelu Historie má být povinné? U některých dat chybí.

Ano, má být. Budeme muset zjistit, kde to chybí. Mám dojem, že jsem to už řešil, tak je to divné.

To samo platí pro pole nazev u modelu Kladyzm

To je zjevně nekompletní migrace, protože pole tam vůbec nemá být: https://github.com/ARUP-CAS/aiscr-webamcr/blob/0a3d438af404368523cc0bcbf218d6f6f5a5889c/migrace/chyby_dat.sql#L23

pro pole soubor u modelu Soubor

Žádné takové pole neexistuje. Co máte přesně na mysli?

a pro pole paginace modelu ExterniOdkaz`

To nemá být not null.

Chybějící definiční bod u okresu je podle mě chyba dat.

Ano, to víme (bude se řešit v #372).

Dále je chyba u tabulky notifikace_typ chybí cesta k šabloně, ale to též považuji za chybu dat.

To asi bude chyba dat, ale to pole je spíš zbytečné. Viz poslední test v #389

  • U modelů, které nemají výchozí primární klíč, bude potřeba vymazat nepoužívané sloupce ID.

Nevím, co máte na mysli. Primární klíč by myslím měly mít všechny tabulky kromě těch v 1:1 vazbách.

  • U modelu PianSekvence jsou duplicity.

To bude nějaké chybné nastavení. Každá ZM50 je tam dvakrát, jednou pro katastry a jednou pro běžné PIAN. Klíč pian_sekvence_kladyzm_id_key tam nemá být.

  • Hash hesel přidám do skriptu na kopírování dat, abychom na to nezapomněli.

Bezva.

pesikj commented 1 year ago

@motyc

Žádné takové pole neexistuje. Co máte přesně na mysli?

Mělo tam být vytvoreno.

motyc commented 1 year ago

@pesikj To jsme smazali (data jsou přesunuta do historie).

motyc commented 1 year ago

@pesikj Porovnal jsem vůči sobě DB test_migrace_20230207 a django_migrated_db. Výsledek jsem prošel a nechal v něm pouze to, co je skutečně relevantní rozdíl mezi oběma DB. V principu je hlavní problém v tom, že u FK constraints není v django_migrated_db správně nastaveno kaskádování (nejde o názvy, ale o ON EDIT/DELETE). Pak tam ještě chybí ty trigger funkce a je tam i několik dalších rozdílů. Vše je podrobně vypsáno zde: srovnani_Django_models.txt

U toho kaskádování bych ještě zmínil, že někde používáme "NO ACTION" a někde "RESTRICT". Neměl by v tom být významný rozdíl, ale otázka je, zda to nějak nesjednotit. Rozdíl by asi měl být v tom, zda se to aplikuje pro statement ("RESTRICT") nebo až pro celý commit ("NO ACTION"), ale vlastně sám nevím, co je vhodnější.

K tomu je třeba doladit jednu drobnost - máte nově nastaveno auth_user.jazyk (resp. uzivatel_historicaluser.jazyk) jako NOT NULL. To mi vůbec nevadí, ale nevím, jestli to je kompatibilní s migrovanými daty. Asi tam pole nebude defaultně vyplněno. Patrně by to tam chtělo mít také default na češtinu.

pesikj commented 1 year ago

@motyc Osobně bych navrhoval používat pouze RESTRICT. Sice nevím, zda je v aplikaci nějaká konkrétní akce, kde by byl rozdíl mezi RESTRICT a NO ACTION, ale v případě komplikovanějších vazeb nebude RESTRICT blokovat mazání neoprávněně.

motyc commented 1 year ago

@pesikj Ok, pak tedy všechny "NO ACTION" lze generálně změnit na "RESTRICT".

motyc commented 1 year ago

@pesikj Při revizi issues jsem narazil na #680, což je ale věc, která je víceméně hotova, resp. na úrovni migrovaných dat je vše připraveno. Smazal jsem tedy pomocí https://github.com/ARUP-CAS/aiscr-webamcr/commit/0dc1806c1b048554c5db8b8944ea392527c1e76e dané pole z tabulky. Jen na to upozorňuji, abyste to ještě prosím promítl do modelů. Nevím, zda rovnou dokážete vyřešit i související nutné úpravy aplikace (měly by to snad být drobnosti), každopádně vazbu na dané issue zapíšu i do úvodního příspěvku.

motyc commented 1 year ago

@pesikj Pár drobných oprav zahrnuto ještě do https://github.com/ARUP-CAS/aiscr-webamcr/commit/ece662a15bc1f781ebf1c21b503b479336ab8b6f

pesikj commented 1 year ago

@motyc Tady jsou zpracované rozdíly. Zkoušel jsem porovnání obou databází v PgAdmin, ale není to moc spolehlivé. Např. u triggerů to hlásí rozdíly, i když jsou v obou databázích totožné triggery.

srovnani_Django_models (1).txt

motyc commented 1 year ago

Výsledek testu

@pesikj Do SQL migrací již nebudeme zasahovat, s výjimkou úpravy triggerů, nebo v případě nových potřeb. DB test_migrace_20230223 je v pořádku a drobné ladění lze řešit již jen na úrovni modelů. Výseldek testu říká, co je třeba upravit pro django_migrated_db.

Obecné

Úpravy modelů (vztaženo k https://github.com/ARUP-CAS/aiscr-webamcr/tree/feat/migrace-841)

Chyby FK

Upravit triggery spoléhající na cascade v DB Protože nově nemáme v DB kaskádování, je třeba tyto čtyři funkce upravit tak, aby nejdříve došlo ke smazání potomků

Známé budoucí úpravy

365

372

pesikj commented 1 year ago

@motyc

Upravit triggery spoléhající na cascade v DB Protože nově nemáme v DB kaskádování, je třeba tyto čtyři funkce upravit tak, aby nejdříve došlo ke smazání potomků

delete_connected_documents (potomci: dokument_cast, neident_akce, neident_akce_vedouci, komponenta_vazby, komponenta, komponenta_aktivita, nalez_objekt, nalez_predmet) delete_history_spoluprace (potomci: historie_vazby, historie) delete_related_komponenta (potomci: komponenta_vazby, komponenta, komponenta_aktivita, nalez_objekt, nalez_predmet) delete_related_soubor (potomci: soubor_vazby, soubor)

Je opravdu nutné toto implementovat? Kaskádování bude zajištěno aplikací, která smaže všechny navázané záznamy dle referenční integrity na modelech.

motyc commented 1 year ago

@pesikj Jenže tady referenční integrita v běžném smyslu slova nefunguje, kvůli N:M vazbám v mezilehlých tabulkách. Alespoň tak by to bylo v relační DB, ale nevím, jak se v tomto bude chovat Django. Určitě je pravda, že od určité úrovně už by mělo kaskádování fungovat v rámci hierearchie, ale nebude patrně fungovat na té první úrovni. Navíc ono my ani nechceme, aby se všude např. mazala historie - tu mažeme právě specificky v případě spolupráce (delete_history_spoluprace); např. při smazání akce či lokality má v tabulce historie zůstat a je to tak dobře.

Co je třeba také nějak řešit bez ohledu na kaskádování je delete_connected_documents, protože tam je ta podmínka spuštění úplně specifická.

pesikj commented 1 year ago

@motyc Byl byste ochoten udělat samostatnou kontrolu struktury databáze bez triggerů, nebo to chcete celé najednou?

motyc commented 1 year ago

@pesikj To by neměl být problém, klidně to zkontroluji zvlášť. Dejte jen vědět, kterou DB přesně a kdy to bude ke kontrole.

jnihnat commented 1 year ago

triggery upraveny.

pesikj commented 1 year ago

@motyc Migrovaná databáze (včetně triggerů) byla vytvořena pod názvem django_migrated_db.

motyc commented 1 year ago

Výsledek testu

Úprava trigger funkcí - korekce

Obecné

pesikj commented 1 year ago

Byly upraveny modely a vytvořeny migrace, které umožňují vygenerování databáze na základě definic modelů. Migrace byly upravovány ručně tak, aby byl minimalizován jejich počet.

jnihnat commented 1 year ago

@motyc pro jistotu pred tim nez to predelam, jak ma fungovat mazani arch zaznamu a dokumentu?:

motyc commented 1 year ago

@jnihnat Pokud mažu archeologicky_zaznam, tak:

Pokud mažu dokument přímo, kaskáduje se normálně (všechny části dok., neident. akce, komponenty dokumentu a nálezy dokumentu)

jnihnat commented 1 year ago

opraveno teda v #953 Ohledne testu, ono to lokalne funguje, akurat otestovat to na novo migrovane DB sem nemohl, uz ste @motyc a @pesikj skouseli i migraci dat do novo vytvorene DB? Ja skousel obycejny dump a restore dat ale to mi neproslo, tak nevim ci uz na to mate postup.

motyc commented 1 year ago

@jnihnat To je krok, na který se teď pokud vím chystá @pesikj, ale nevím, jak přesně pokročil. Ještě je tam třeba udělat těch pár oprav výše a pak se to snad může vyzkoušet.

pesikj commented 1 year ago

@motyc

  • neident_akce_vedouci - doplnit chybějící FK constraint mezi neident_akce_vedouci.neident_akce a neident_akce.dokument_cast (ON UPDATE CASCADE, ON DELETE CASCADE) - původní zmizel kvůli odstranění PK v neident_akce

Jak to je přesně myšleno? Jako že má být přímo provázána tabulka neident_akce_vedouci a dokument_cast?

pesikj commented 1 year ago

@motyc ¨

  • soubor - path SET TYPE text - opomenuto

Jde o FileField, navíc, vzhledem k přechodu na Fedoru, to budeme muset předělat, navrhuji nyní neřešit.

motyc commented 1 year ago

@pesikj ad https://github.com/ARUP-CAS/aiscr-webamcr/issues/841#issuecomment-1554955947

ad https://github.com/ARUP-CAS/aiscr-webamcr/issues/841#issuecomment-1554980461

pesikj commented 1 year ago

@motyc

  • django_migrated_db -> opět přejmenovat na vzor_20230425 a uchovat jako předlohu pro další testy;

Hotovo, django_migrated_db je nová verze.

  • všude nastavit SELECT permissions pro cz_archeologickamapa_api_view (s tímto účtem se nezobrazí ani SQL pro: django_content_type, auth_permission, všechny sekvence atd.) - asi opomenuto

Hotovo

  • potřebujeme tabulky django_cron_cronjoblock a django_cron_cronjoblog, když nově používáme Celery?

Jsou odebrány.

  • pole hlavni_autor nakonec nebude potřeba? Pokud to tak je, měli bychom jeho vytváření odstranit z SQL migrací... To mohu udělat, ale prosím o potvrzení, že to skutečné má zmizet

@jnihnat potvrdil, že pole není potřeba

  • neident_akce_vedouci - doplnit chybějící FK constraint mezi neident_akce_vedouci.neident_akce a neident_akce.dokument_cast (ON UPDATE CASCADE, ON DELETE CASCADE) - původní zmizel kvůli odstranění PK v neident_akce

Tady nevím, proč se to děje, v migracích to je správně

('neident_akce', models.ForeignKey(db_column='neident_akce', on_delete=django.db.models.deletion.CASCADE, to='neidentakce.neidentakce')),
  • organizace - organizace_mesicu_do_zverejneni_check: (mesicu_do_zverejneni >= 0 AND mesicu_do_zverejneni <= 1200) - opomenuto

Zde se propisuje jen podmínka na 0. Nevím, zda je to chyba či zda se validátory na úroveň databáze nepropisují a je potřeba to dopsat ručně.

  • pian_sekvence - UNIQUE (kladyzm_id, sekvence, katastr) - opomenuto

Vyřešeno

    CONSTRAINT pian_sekvence_kladyzm_id_sekvence_katastr_1b812006_uniq UNIQUE (kladyzm_id, sekvence, katastr),
  • soubor - path SET TYPE text - opomenuto

Domluveno, že nebudeme nyní řešit.

  • ruian_kraj.nazev_en - patrně omylem došlo k úpravě nastavení pole - má být character varying(100) NOT NULL + UNIQUE (tak to bylo a nyní jen prostý text bez dalšího)

Opraveno.

Prosím o kontrolu výše uvedeného plus test opravených triggerů od @jnihnat

motyc commented 1 year ago

Výsledek testu

Zbývá dořešit

pesikj commented 1 year ago

@motyc

ALTER TABLE IF EXISTS public.ruian_katastr ALTER COLUMN definicni_bod DROP NOT NULL;

Ano, to je skutečně chyba.

hashování hesel otestovat reálné nasazení pod Django aplikací (fungování triggerů otestujeme za běhu tam)

Řešení dnes budu nasazovat

motyc commented 1 year ago

@pesikj Prošel jsem všechny tabulky, a účet cz_archeologickamapa_api_view nemá SELECT práva ještě na následující:

django_celery_beat_clockedschedule
django_celery_beat_crontabschedule
django_celery_beat_intervalschedule
django_celery_beat_periodictask
django_celery_beat_periodictasks
django_celery_beat_solarschedule
django_celery_results_chordcounter
django_celery_results_groupresult
django_celery_results_taskresult
django_session
lokalita
nalez_objekt
nalez_predmet
neident_akce
neident_akce_vedouci
notifikace_projekty_pes
oznamovatel
motyc commented 1 year ago

@pesikj @jnihnat Prosím o dořešení https://github.com/ARUP-CAS/aiscr-webamcr/issues/841#issuecomment-1561030378 a https://github.com/ARUP-CAS/aiscr-webamcr/pull/996, ať to můžeme konečně uzavřít.

pesikj commented 1 year ago

@motyc Vrátím se k tomu po dodání další testovací verze Fedory

motyc commented 1 year ago

@pesikj Ještě doplňuji:

Důvod patrně odhlen v https://github.com/ARUP-CAS/aiscr-webamcr/issues/841#issuecomment-1717781809

motyc commented 1 year ago

@pesikj Issue jsem pročistil skrytím starých komentářů. Našel jsem při ladění migrací nějaké nejasnosti v souboru https://github.com/ARUP-CAS/aiscr-webamcr/blob/dev/migrace/copy_data.py. Jinak je myslím vše již ok, resp. případné problémy zjistíme až při dalším pokusu o kompletní migraci. Po vyřešení problémů níže toto uzavřu.

https://github.com/ARUP-CAS/aiscr-webamcr/blob/641e0db5dc91946fa87b447d2d7d99e411455cad/migrace/copy_data.py#L38

https://github.com/ARUP-CAS/aiscr-webamcr/blob/641e0db5dc91946fa87b447d2d7d99e411455cad/migrace/copy_data.py#L222

motyc commented 9 months ago

@pesikj

Tu sekvenci přece taky potřebujeme nastavit, nebo ne?