search

keboola / db-writer-snowflake

Snowflake database writer
MIT License
0 stars 1 forks source link

Swap tables on full load #55

Closed Halama closed 5 years ago

Halama commented 5 years ago

Předejde situaci kdy byla cílová tabulka během full loadu prázdná. Loadne do staging tabulky a nakonci udělá atomický swap tabulek.

Halama commented 5 years ago

Koukám že se tam vůbec nikde netestuje celý load a ověření že data jsou skutečně nahraná?

Halama commented 5 years ago

Co má testovat třeba tohle https://github.com/keboola/db-writer-snowflake/blob/master/tests/phpunit/SnowflakeTest.php#L254 ? writeAsync mi nějak v kontextu toho writeru nedává smysl.

ErikZigo commented 5 years ago

Proc to delas, kdyz na tom delam od rana ? :(

Halama commented 5 years ago

chtěl jsem vidět jestli to pojede a tohle bylo nejrychlejší.

Halama commented 5 years ago

to můžeš dotáhnout nebo použít to tvoje to je jedno.

Halama commented 5 years ago

teď mě ale překvapili ty testy, přijde mi že se to tam nikde netestuje celé?

Halama commented 5 years ago

tak to pushni co máš

Halama commented 5 years ago

Přijde mi že aby to šlo otestovat co to loaduje tak je to potřeba vlastně tohle celé tělo metody https://github.com/keboola/db-writer-snowflake/blob/351aba4e539cb482ec8136f976683dbb37d9e1c0/src/Keboola/DbWriter/Snowflake/Application.php#L85 vykopírovat do testu. podobně jako tady https://github.com/keboola/db-writer-snowflake/blob/08180855a0743aaeadf0b5ca098e2b163a37b6fb/tests/phpunit/SnowflakeTest.php#L254

Ale proč ne asi pro teď. Tím pádem vůbec nevím jestli ta úprava výše funguje, myslel jsem že to je pokryté.

Halama commented 5 years ago

Nechávám to teď být teda. Platí teda ale že releasneme verzi kde se budou swapovat tabulky při full loadu.

ErikZigo commented 5 years ago

Co má testovat třeba tohle https://github.com/keboola/db-writer-snowflake/blob/master/tests/phpunit/SnowflakeTest.php#L254 ? writeAsync mi nějak v kontextu toho writeru nedává smysl.

To testuje tu Writer::writeFromS3() metodu. Jinak mas pravdu, chybi tam testy toho co provede Applicatin::runAction

ErikZigo commented 5 years ago

Do funkcionalniho testu jsem pridal ostestovani naimportovanych dat.

A pro ty nove metody createIfNotExists + swapTables jsem dopsal testy.

Halama commented 5 years ago

To vypadá dobře! je to tím pádem na R4R nebo tam ještě něco ladíš?

ErikZigo commented 5 years ago

Podle me na review. Ja jsem akorat doplnil test na ty data po importu a nove metody ktere jsi udelal ve Writer classe.

Halama commented 5 years ago

@ErikZigo mergneš to a releasneš prosím. Koukám že to nemá testovací konfiguraci tak by bylo fajn předtím připravit nějakou s full loadem a vyzkoušel ji předtím a po releasu.

Halama commented 5 years ago

připravil jsem tady https://connection.keboola.com/admin/projects/4088/jobs/467726206

ErikZigo commented 5 years ago

Zrovna u tohodle writeru to od jiste doby tak, ze nez to nasadim tak si to deploynu rucne jako testovaci komponentu a tu zkousim pustit.

Halama commented 5 years ago

tak vlastně stačí ho otagovat a pustit si tag

Halama commented 5 years ago

a není potřeba deploy ručně

Halama commented 5 years ago

hmm, s tím tagem kecám ono to má udělané jinak https://github.com/keboola/db-writer-snowflake/blob/master/deploy.sh než normálně https://github.com/keboola/component-generator/blob/7f52335d629180c63481533ef751447e89e93a75/templates-common/deploy.sh#L24 Stačilo by upravit a půjde to.

Halama commented 5 years ago

^ to imho bude nejjednodušší udělat takhle a bude to alespoň konzistentní s novými komponentami.

ErikZigo commented 5 years ago

@Halama https://github.com/keboola/db-writer-snowflake/pull/56