-
Notifications
You must be signed in to change notification settings - Fork 0
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
Import files abs to snflk #1
Conversation
jdu to kouknout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nahodil jsem si to ale zatím z toho asi fakt moc nevykoukám začátek imho dobrý. Počkal bych až to bude implemenovat celý ten importní flow. Zajímalo by mě jak to bude pak řešeno architektonicky, doteď to bylo přes https://refactoring.guru/design-patterns/template-method ale pak se to tam trochu zkomplikovalo více typy CSV importů.
Import jako ze souboru tak ze snowflake by měl být funkční. Teď "nakopíruju" ty testy z púvodní lib, jak to bude fakčit tak dodělám unit testy a pustím sa do exportu. |
@Halama kdybys měl chvilku mrkni prosím tady na skipped testy jestli je ok to co sem všechno vyhodil. |
Asi bych to hodil RfR a nechal export do vlastního PR. Už tak je to mega velké. |
Zkusím to dnes nebo zítra zkouknout. |
Jdu na to mrknout. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vypadá to super, nebál bych se to releasnout. Doladí se to při integraci do KBC v separátních PR kdyby se něco objevilo. Pořád platí že do KBC se tenhle nový importer bude v první fázi používat jenom pro ABS importy ze SNFLK takže by nemělo hrozit že to rozbije něco existujícího. viz. https://github.com/keboola/connection/issues/2015
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potvrzuju že po tom fixu už ma jedou všechny testy i lokálně.
php-db-import