Skip to content

12th commit #1

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.

Sign up for GitHub

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

Jump to bottom
Merged
igormurigor merged 1 commit into from
Oct 5, 2023
Merged

12th commit #1

igormurigor merged 1 commit into from
Oct 5, 2023

Conversation

@igormurigor
Copy link
Owner

@igormurigor igormurigor commented Oct 5, 2023

No description provided.

@igormurigor igormurigor merged commit 494e6ee into main Oct 5, 2023
@TimBogevich
Copy link
Contributor

TimBogevich commented Oct 6, 2023

Привет высылаю обратную связь

  1. PR оформлен не красиво, посмотри любой PR любого проекта в гитхабе
    image
    Вот пример Implements AwsBaseOperator and AwsBaseSensor apache/airflow#34784. Все изменения оформлены одним коммитом скоупом.
  2. DWH. Не понятен ответ "В аптечном бизнесы могут быть таблицы фактов продаж, инвернтаризации
    или таблицы клиентов или самого лекартства как продукта". Что из этого измерения?
  3. ETL 1. Данный код можно сделать гораздо более компактным, write_sql например
  4. etl_2 - тоже всю загрузку можно было бы сделать через pandas, код был бы гораздо компактнее
  5. Юнит тест очень не репрезентативный причем один кейс только. Я бы сказал что это проверка качества данных. Это не юнит тест

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@igormurigor @TimBogevich