Rozšířit současné chování table manifestů i na soubory

tomasfejfar commented 6 years ago

Manifest tabulek se aktuálně může dělat třemi způsoby:

zápis s parametry

public function writeTableManifest(
string $fileName,
string $destination = '',
array $primaryKeyColumns = [],
array $columns = [],
bool $incremental = false,
array $metadata = [],
array $columnMetadata = [],
string $delimiter = ',',
string $enclosure = '"'
)

zápis z pole, který interně používá parametry

public function writeTableManifestFromArray(
string $filename,
array $manifest
)

zápis pomocí options

public function writeTableManifestFromOptions(
string $fileName, 
WriteTableManifestOptions $options
)

U souborů je jen první možnost.

Jak často se v praxi používají soubory?
A jak často se manipuluje s manifesty souborů?
Je to něco, čemu má smysl věnovat čas?
Nebo stačí, že je tam jedna metoda na zápis manifestu?

Druhá věc je, jestli má smysl udržovat tři různé způsoby pro tabulky. Kvůli dalším změnám bude stejně potřeba vydat major v3, takže by dávalo smysl to upravit tak, aby se do toho za chvilku zas nesahalo. Plus to má návaznost na component generátor, který by měl novou verzi používat. Takže bych rád udělal teď větší release a pak to nějakou dobu nechal ležet - podle jak si to sedne a jak na tom lidi budou dělat nové komponenty, tak nasbírat feedback. Jak to děláte u jiných balíčků? Nemám tolik zkušenosti s udržováním opensource balíčků, které by se hodně vyvíjely.

odinuv commented 6 years ago

kdyz mame manifestoptions tak bych klidne zrusil writeTableManifestFromArray a mozna i writeTableManifest, at clovek zbytecne nemusi premyslet nad tim, co pouzit
soubory se pouzivaji malo, ale kvuli konzistenci bych to udelal stejne
compnent generatoru staci zmenit composer.json, ne?
myslim, ze uz si to docela seda :)

tomasfejfar commented 6 years ago

Tím pádem budu muset ještě udělat Options::fromArray(), aby se dalo udělat

$manifest = getManifest();
mutateManifest($manifest);
writeManifest(Options::fromArray($manifest));

odinuv commented 6 years ago

jo? ja to fromarray bral jako workaround pro to aby clovek nemusel zadat 10 parameteru kvuli jednomu nakonci, s tim Options objektem to neni potreba ne?

tomasfejfar commented 6 years ago

No, ale typický příklad format-csv. Musíš natahnout originální manifest a upravit v něm delimiter/encapsulator a uložit. A je otrava+error-prone to dělat ručně.

odinuv commented 6 years ago

aha, ok beru

odinuv commented 6 years ago

ne, neberu

odinuv commented 6 years ago

input manifest je totiz uplne jinej nez output manifest https://github.com/keboola/input-mapping/blob/master/Configuration/Table/Manifest.php https://github.com/keboola/output-mapping/blob/master/Configuration/Table/Manifest.php

coz me privadi na myslenku, ze je vlastne format-csv strasne vadny, protoze se v nem v input manifestu predpoklada neco, co tam nema byt

narazil jsem na to ted z opacne strany u iconv processoru, u kteryho bych chtel, aby manifesty nezahazoval a zjistil jsem, ze prekopirovani manifestu neni vubec jednoduchy, protoze kdyz ho proste zkopiruju, tak to nebude validni out manifest

bud mam vlci mlhu, nebo jsem tohle hodne nedomysleli premyslim, jak z toho vybruslit a napada me tohle:

common interface je svaty
- takze in manifest je vzdy https://github.com/keboola/input-mapping/blob/master/Configuration/Table/Manifest.php a out manifest je vzdy https://github.com/keboola/output-mapping/blob/master/Configuration/Table/Manifest.php
- procesor kdyz chce manifest zkopirovat, musi to udela rucne (mohli bysme pridat manifestManager::copy
- docker runner pro kazdej procesor manifesty prechrousta (takze zase udela z out manifestu in manifest) a to tak ze mergne puvodni in manifest out manifestem, coz by znamenalo rozsirit in manifest of ficury out manifestu, coz mi prijde blby protoze to nedava smysl
jiny reseni by bylo sjednotit manifest kompletne, coz mi prijde blby, protoze 1/2 veci nebude davat smysl
jiny rezeni by bylo rozdelit procesory na in procesory, ktere konzumuji in a produkuji manifesty a out procesory, ktere konzumuji a produkuji out manifesty, to mi prijde ze je v tutochvili asi nejbliz tomu jak jsme to zacali delat

cc @ondrejhlavacek kdyz tak bych dal call

ondrejhlavacek commented 6 years ago

call hotovej, nápad:

v out manifestu jsou teď 2 věci spojený dohromady

metadata tabulky/csv (sloupce, parametry csv, pk, ...)
instrukce pro load do storage (incremental, deletewhere...)

kdybychom to od sebe dokázali odtrhnout, bude manifest univerzální na všechny strany a instrukce pro load se budou přenášet nezávisle

totéž by mohlo platit u souborů (tam jsou ty manifesty od sebe asi nejvíc rozdílný)

tomasfejfar commented 6 years ago

Sorry, mám v tom trochu zmatek - jak se teda stane z "out manifestu" "in manifest"? Jak píšeš, že "docker runner pro kazdej procesor manifesty prechrousta" je možné budoucí řešení, nebo už to takhle teď dělá?

Mě to, že v "in manifestu" jsou jiné hodnoty než v "out manifestu" nijak neuráží. V "in" jsou informace pro čtenáře, v "out" informace pro zapisujícího. Jestli je to tak jak píšeš s tím mergováním - tak ten scénář "kopírování" manifestu vůbec neexistuje. A jestli to správně chápu, tak tvůj scénář s tím, že se jen propíše manifest taky neexistuje - když nic nepošlu, tak se nic nepropíše do "in" a "in" zůstane stejný.

Pokud je to tak, že scénář mergování manifestu je"budoucí řešení", tak to nemusí být tak, že je nějaký array_merge, ale může to být složitější, ne? Vezme si to z manifesu jen ty věci, které jsou v in manifestu a ty někam uloží. Zbytek je ohledně zápisu do storage a to se vyřeší jinde a ty data už dál nejsou potřeba ne?

odinuv commented 6 years ago

zatim se nic nedela

urazet te to nemusi :), ale kdyz vezmes processor-format-csv a zaradis ho na zacatek chainu, tak nebude fungovat, protoze dostane in manifest, misto out manifestu, ktery ocekava. Kdyz vezmes processor-format-csv (a strcis pred nej processor-create-manifest) a zaradis ho pred komponentu, tak ta komponenta najednou dostane out manifest misto in manifestu, ktery ocekava.

(ne)kopirovani manifestu je issue, protoze v manifestu jsou ulozeny table a column metadata (produkovany nebo konzumovany komponentou) a pokud procesor manifest nezkopiruje, tak se ztrati, podobne se muze ztratit incrementalni load a nastaveni pk. Tzn. ted napriklad u processor-iconv neni duvod, aby zmena kodovani zpusobila ztratu veskerych dat. Tam nastesti processor nic s manifestem delat nemusi, takze ho staci jen prekopirovat a funguje spravne i v after i v before chainu. Ale treba processor-add-row-number-column funguje spravne jen v after chainu.

tomasfejfar commented 6 years ago

Jo, takhle! Takže oběcně to vypadá takhle:

START → in manifest → in processor → in manifest → [component] → out manifest → out processor → out manifest → END

V mojí hlavě to doteď fungovalo takhle: START → in manifest → in processor → out manifest → MAGIC → in manifest → [component] → out manifest → MAGIC → in manifest → out processor → out manifest → END

odinuv commented 6 years ago

there is no magic, just cake

ujovlado commented 6 years ago

Tak mi napadlo, nepridali by sme do processoru rovno nazov, ci je je urceny do before/after chainu? T.j. "Post processor" "Pre processor" a "Processor". Alebo blbost? :)

odinuv commented 6 years ago

https://github.com/keboola/php-component/issues/21#issuecomment-374573666

ale rozhodne bych to nekodoval do nazvu, ale udelal to jako vlastnost komponenty

tomasfejfar commented 6 years ago

Pokud ano, tak by to mělo IMO být kontrolováno na úrovni konfigurace. (aha, odin byl rychlejší)

odinuv commented 6 years ago

proto by to mela byt vlastnost komponenty

tomasfejfar commented 6 years ago

Ještě mě napadá: Někdo (?) kopíruje processoru soubory z out zpátky do in pro další processor nebo komponentu. A ten samý aktor by mohl zkonverzovat i manifest, ne? (tj. hrát tu moji "magic") Processor by stejně neměl dělat s manifestem nic, co by se nemohlo zapsat do out, ne? Neměl by třeba měnit datum poslední změny nebo tak. Pokud by processor vygeneroval in manifest místo out manifestu, tak by mu ten aktor prostě ty změny vyignoroval.

Tím by se celá ta logika vyčistila, ne?

odinuv commented 6 years ago

jenze potom bys musel mit v in manifestu veci, ktery tam nepatri a naopak. Napr. pokud na outu je incremental, tak to nejde dat do in manifestu. Kdyz pridame do in manifestu incremental, tak co to bude na vstupu znamenat? proto ten navrh https://github.com/keboola/docker-bundle/issues/257 tim by mozna vznikl in/out prenositelny manifest a neprenositelna instrukce

tomasfejfar commented 6 years ago

A měl by processor moct měnit incremental? Je pro to nějaký use-case?

Navíc, když to oddělíme, tak vlastně vyrefaktorujeme 50% problému (společné hodnoty v in/out manifestech) do samostatného souboru, ale zbylých 50% (tj. všechny ty ostatní věci v manifestech) stejně zůstanou. A stejně to budeme muset řešit.

odinuv commented 6 years ago

prave ze neni, ale pokud ho povesis za extractor, kterej nastavil incremental, tak se ta hodnota nesmi zratit, a to by se stalo, pokud bys prekonvertoval out manifest na in manifest

jo, coz ve me podporuje to, ze by bylo nejlepsi mit in processory a out processory (pripadne obojetny, pokud na manifest vubec nesahaji)

ondrejhlavacek commented 6 years ago

podle mě by extraktor vůbec neměl řešit, jestli je load do storage incremental nebo ne, nebo jaký se maj smazat data v tabulce před loadem. to není jeho byznys. může znát sloupce, primární klíče, metadata, nastavení csv, to všechno může patří.

takže co bych ideálně udělal je, že by vznikl processor-load-instructions, kterej by byl vždy poslední v celým chainu (u komponenty co zapisuje do storage) a někam by zapsal, že tabulka se nahrává inkrementálně a parametry delete_where_*.

teď můžeme udělat, že to nahackuje do existujícího manifestu (za předpokladu, že už za ním nic nepoběží), a tím se manifest stane univerzální (až do tohodle procesoru). pokud by to mohl zapsat někam jinak, bylo by to čistší.

odinuv commented 6 years ago

extraktor to resit muze, protoze inkrementalnost je povaha dat - muzes z databaze extrahovat dve tabulky, jednu inkrementalne a druhou ne, protoze to u druhe proste nejde.

procesor-load-instructions je podle m konina protoze:

neni univerzalni - viz predchozi (sice by to slo obejit pres config rows, ale proc obchazet problem, kterej jsme jeste nevytvorili)
je povinny
musel by byt na nejakym specifickym miste v konfiguraci
musel by se pridat do mraku existujicich konfiguraci

odinuv commented 6 years ago

a hlavne bysme jim nic neziskali, pokud bysme opravdu meli delit manifest od load-instructions (LI), tak

extractor zapise manifest k souboru
extractor zapise LI k souboru
pri spusteni procesoru (prehozeni in/out) se prehodi vsechno krome LI
cili dalsi procesor uvidi soubory a manifsety
LI bude mit docker runner u sebe a uz je nikam neda
stejnym zpusobem by to bylo na IM, tam se oddeli manifest a storage-data (ty dostane pouze komponenta, ne processor)

ale jestli to bude k necemu uzitecne nevim

ondrejhlavacek commented 6 years ago

rozlišuju

1) inkrementální load do Storage 2) stažení části dat ze zdroje podle nějakých pravidel, zapamatování si, kde jsi skončil, a pokračování při dalším runu

najdeš validní use casy pro libovolnou kombnaci těchle přepínačů a nemůžeš ani o jedný z nich říct, že je to edge case. pro mě jsou to dvě úplně odlišný věci, který by sis měl nakonfigurovat odděleně.

neni univerzalni - viz predchozi (sice by to slo obejit pres config rows, ale proc obchazet problem, kterej jsme jeste nevytvorili)

procesory jsou s config rows spíš spjatý, než že by byl záměr, aby mohly fungovat bez nich

je povinny

ano, stejně tak je povinný u S3 extraktoru processor-create-manifest, pokud chceš třeba říct, jaký má tabulka sloupce, delimitery nebo inkrementální load. ale komponentu můžeš spustit bez něj a stejně to udělá nějakej výstup.

musel by byt na nejakym specifickym miste v konfiguraci

ano, stejně jako jakejkoliv jinej procesor. pořadí procesorů není libovolný, stejně jako pořadí příkazů v pipe commandlajny. pokud ti jde o to, že musí bejt vždy poslední, stačilo by, aby procesory, který s procesorama nic nedělají a jen je přenesou, začaly přenášet i load instructions

musel by se pridat do mraku existujicich konfiguraci

nemusel. může to celý bejt udělaný se zpětnou kompatibilitou, to platí pro jakýkoliv řešení, který vymyslíme

ondrejhlavacek commented 6 years ago

https://github.com/keboola/php-component/issues/21#issuecomment-375043925

aha! myšlenka, že by se load instructions vůbec nepropagovaly do procesorů se mi líbí. imho by to neměl ani zapisovat extraktor/aplikace. docker runner by si to nějak mohl udržovat u sebe a aplikovat to až na výsledek.

odinuv commented 6 years ago

najdeš hromady validní use casy pro libovolnou kombnaci těchle přepínačů a nemůžeš ani o jedný z nich říct, že je to edge case. pro mě jsou to dvě úplně odlišný věci, který by sis měl nakonfigurovat odděleně.

imho se na to divas moc optikou generickych ex (s3, ftp), pokud ale ten extractor vi, co je to za data, tak vi jestli je mozny je loadovat inkrementalne nebo ne, coz ten procesor nebude vedet nikdy

pořadí procesorů není libovolný, stejně jako pořadí příkaz

neni, ale tenhle by mohl byt na jednom jedinnym miste a musel by tam byt vzdycky (kdyz na chvili zapomenu, ze incremental ma nejakou default hodnotu)

btw, mit procesor, kterej zapise do souboru boolean flag me prijde jako slusnej overkill :)

ondrejhlavacek commented 6 years ago

imho se na to divas moc optikou generickych ex (s3, ftp), pokud ale ten extractor vi, co je to za data, tak vi jestli je mozny je loadovat inkrementalne nebo ne, coz ten procesor nebude vedet nikdy

vědět to má uživatel, konfigurace a docker runner. všichni ostatní jsou v tom zbytečně.

btw, mit procesor, kterej zapise do souboru boolean flag me prijde jako slusnej overkill :)

větší overkill je na to mít komponentu nebo nutit každou komponentu, ať to dělá, hlídat to a testovat to.

tomasfejfar commented 6 years ago

Napsal jsem to, ale ve výsledku jsem zjistil, že jsem tím nic nevymyslel :D Ale nechám to tady JFYI jak jsem přemýšlel

Když to vezmu z nezasvěceného pohledu:

mám něco co mi tahá data z mého eshopu (dejme tomu http-ex stahující CSV)
když to nastavuju, tak vím, že mi to vrací všechny
můžu chtít vyfiltrovat jen ty, co nejsou hotové a už jsou zaplacené - čekal bych, že tam zaškrtnu "filter results" a vyplním tam filtr (tj. dám processor-filter-data (where: "status != closed AND paid = true"))
když to nastavuju, tak vím, co mi z toho vypadne a tím pádem zaškrtnu pod tím ještě checkbox "incremental", ať mi to uloží jen nové podle PK status_change_id (jen nové změny stavů)

Co mi z toho nahoře vyplynulo:

představuju si, že lidi dělají většinu věcí v Keboole v UI, jako já na hackatonu
umím si představit asi jen jednoduché scénáře
přemýšlím nad tím jako nad bashovým onelinerem, takže transformace = grep/awk a tím pádem vlastně moc nechápu, proč by měl být někde v extractoru nějaký externí "where"
```
extractor       -->   filter/transformator --> writer
curl audit.log   |     grep -v user:admin   >  users_audit.log
```

ha, teď když jsem si to celé napsal, tak jsem trochu líp pochopil


<-------------------- extractor ------------------->
curl audit.log | grep -v user:admin > users_audit.log

<---------------------tranformace----------------> cat user_audit.log | awk ... > today_user_audit.log

<--------------------writer---------------------------> cat today_user_audit.log | jq | nc localhost:123



----

**Takže vlastně když to shrnu, tak z toho mám pocit, že by zápis do storage měl řešit jen `> vs >>` (tj. zapiš nebo jen přidej nové). A pokud někdo chce filtrovat, tak by to mělo být v processoru.**

Ničím dalším k diskusi asi nepřispěji. Sorry, že je to tak dlouhé, ale bylo to dlouhé zamyšlení a chtěl jsem si to sám utřídit. A třeba se tady k tomu ještě vrátím. A třeba to pomůže i vám líp pochopit, jak nad tím přemýšlejí lidé, kteří s tím mají míň zkušeností... ;)

ondrejhlavacek commented 6 years ago

Bacha, vypadá to, že každej myslíme jiný filtrování. To delete_where_*, o kterým mluvíme, nemaže data v CSV, který se uploaduje do Storage, ale přímo ve Storage pomocí https://keboola.docs.apiary.io/#reference/tables/manage-table-rows/delete-table-rows.

Používá se na to (a ne moc často), když chceš vyměnit jen nějakou část dat (např. podle nějakýho atributu). Napříkad máš 2 db, který mají stejnou tabulku (strukturu), ale mají jiný obsah. Každá z těch tabulek má např. sloupec type, první db má hodnotu db1, druhá db2.

Když to loaduješ do Storage, nemusíš to mít ve dvou tabulkách ve Storage a pak to nějak unionovat v transformaci, ale před loadem loadu prvního CSV, kde type=db1 zavoláš

delete_where_column = type
delete_where_operator = eq
delete_where_values = [db1]

a pak to CSV loadneš.

Je to poměrně zastaralej koncept a teď bychom spíš řekli, že to máš mít ve dvou tabulkách a pak si to někde najoinovat. Ale používá se to i v transformacích, kde si můžeš díky tomu oddělit od sebe kus pipeline, kterou procházejí jiný data, co se maj zapsat do jednoho výstupu.

tomasfejfar commented 6 years ago

Asi to ještě pořád nechápu. Jaká je tam výhoda a motivace toho, že se ty data mažou až na Storage? Přijde mi, že je to jen víc kryptické a půjde to blbě debugovat (nebude verze před a po, ale jen jedna "po"). Proč to někdo používá a neudělá si to v transformaci?

ondrejhlavacek commented 6 years ago

Souhlas, je to premature optimization, co si neseme z minulosti. Můžem to vyhlásit jako deprekovaný, ale bude to docela bolet, než to vymlátíme. Ale teď to dobře slouží k ilustraci prolému :-).

odinuv commented 6 years ago

no i kdyz budeme ignorovat delete_where, tak nam tam zustava incremental

ondrejhlavacek commented 6 years ago

Přesně

tomasfejfar commented 6 years ago

Zavřením #25 se řeší předmět téhle issue, takže zavírám. Pokračovat v obecné diskusi by se asi dalo v https://github.com/keboola/docker-bundle/issues/257

keboola / php-component

Rozšířit současné chování table manifestů i na soubory #21