Add `TransformAndCatalog` flow #720

Rafalz13 · 2023-07-14T09:04:45Z

Summary

It is a need to use DBT in viadot 1.0. DBTTask and CloneRepo tasks are migrated from viadot 2.0. Also, TransformAndCatalog flow is built using viadot 1.0 structure.

Importance

To be able to use the DBT using viadot 1.0.

Checklist

This PR:

follows the guidelines laid out in CONTRIBUTING.md
links relevant issue(s)
adds/updates tests (if appropriate)
adds/updates docstrings (if appropriate)
adds an entry in CHANGELOG.md

angelika233

Added some comments

angelika233 · 2023-07-25T13:48:08Z

requirements.txt

-azure-identity==1.7.1
-great-expectations==0.14.12
+azure-identity>=1.10.0, <2.0.0
+great-expectations>=0.15.50, <0.16.0


why are you specifying a range of versions? wouldn't it be better to set version to specific one?

Changed great-expectations to 0.15.50

angelika233 · 2023-07-25T13:48:11Z

tests/unit/tasks/test_luma.py

+
+def test_luma_ingest():
+    task = LumaIngest(
+        credentials_secret="luma-dev",


most of the time we specify credentials in uppercase

angelika233 · 2023-07-25T13:54:05Z

viadot/flows/transform_and_catalog.py

+        Args:
+            dbt_project_path (str): The path to the dbt project (the directory containing
+                the `dbt_project.yml` file).
+            name (str): The name of the Flow. Defaults to "Transform and Catalog".


Specifying flow name won't cause later issues? for example, it's easier to overwrite the flow.

angelika233 · 2023-07-25T13:57:58Z

docker/Dockerfile

 RUN pip install --upgrade pip
 RUN pip install -r requirements.txt
+RUN pip install -r requirements-dev.txt


are both requirements files needed or the dev one is used only for certain cases? if so you can add if statement for certain tag ie dev

requirements-dev line is removed from Dockerfile

viadot/flows/transform_and_catalog.py

angelika233 · 2023-07-25T14:01:11Z

viadot/flows/transform_and_catalog.py

+            stream_output=True,
+        ).bind(flow=self)
+
+        test_select = self.dbt_selects.get("test", run_select)


maybe change the name from "test" to sth different, test suggests that it's something just fore testing not production

It was migrated from 2.0. I will ask how it can be changed.

angelika233 · 2023-07-25T14:01:58Z

viadot/tasks/git.py

+from prefect.utilities.logging import get_logger
+from prefect.utilities.tasks import defaults_from_attrs
+
+from viadot.task_utils import *


Is it possible to avoid using *?

There was an issue with the circular import

trymzet · 2023-08-02T11:56:08Z

@Rafalz13 I think we also need the _viadot_source column to be added as part of metadata fields added to each dataset ingested by viadot in to_df() method of the base Source class (eg. like the decorator approach in viadot 2).

Rafalz13 · 2023-08-09T14:24:57Z

@trymzet function is added to utils (#735). Users can add a decorator to source class function. For now, adding this decorator to Source class is impossible because of the different structure of the code.

trymzet · 2023-08-10T12:21:09Z

@trymzet function is added to utils (#735). Users can add a decorator to source class function. For now, adding this decorator to Source class is impossible because of the different structure of the code.

Hm I didn't udnerstand that.

You can still add it to the base class; then only the custom sources will need to be modified to use the util function (or just use the to_df() api).

The way you did this, you need to modify all Source subclasses, even the standard ones] with the to_df() method.

trymzet · 2023-08-11T10:35:48Z

@trymzet function is added to utils (#735). Users can add a decorator to source class function. For now, adding this decorator to Source class is impossible because of the different structure of the code.

Ok created issue to document this #737

tests/integration/tasks/test_luma.py

trymzet · 2023-08-30T09:30:12Z

viadot/utils.py

@@ -18,7 +20,7 @@
 from .exceptions import APIError
 from .signals import SKIP

-logger = logging.get_logger(__name__)
+logger = get_logger(__name__)


I don't think this should be a Prefect logger? These utils are a the viadot level; Prefect utils are in a separate file.

viadot/tasks/luma.py

tests/integration/tasks/test_luma.py

viadot/flows/transform_and_catalog.py

trymzet

LGTM

Rafalz13 added 18 commits July 14, 2023 10:32

✨ Added CloneRepo task

90205c3

✨ Added DBTTask task

382e00f

✨ Added TransformAndCatalog flow

f52e41f

✨ Updated init files

c0056e1

📝 Updated Changelog

713cd81

✨ Added shell_run_command function

8cff77b

✨ Added shell_run_command function

ec7b404

✨ Added pygit2 to requirements

af90c40

🐛 Added self to variable name

c90b406

✨ Added LumaIngest task

591f0b1

📝 Updated Changelog

840e032

♻️ Replaced DBTTask and LumaIngest with ShellTask

6b26526

🔥 Removed luma.py and dbt.py tasks

7b229d7

🔥 Removed shell_run_command function from utils

1a49648

🔥 Cleaned up __init__.py file

896578a

♻️ Added and removed libraries

8e85e4d

🐛 Modified requirements

e8433be

📝 Updated Changelog and docstring

687a7b4

Rafalz13 requested review from trymzet and angelika233 July 24, 2023 14:12

trymzet and others added 3 commits July 25, 2023 10:42

✨ Add Luma task

8e50ab0

📝 Added docstring to LumaIngest task

07e1e2e

♻️ Added LumaIngest task

27dfc8a

angelika233 requested changes Jul 25, 2023

View reviewed changes

⚡️ Added source_freshness task

ed9c689

trymzet changed the title ~~Migrate DBT related code to viadot 1.0~~ Add TransformAndCatalog flow Aug 4, 2023

Rafalz13 and others added 2 commits August 7, 2023 14:35

🚧 Updated requirements

9fc4234

Merge branch 'dev' into dbt_to_viadot_1

bfb1e7d

Changed name parameter to required

8943dc2

🚧 Updated command for LumaIngest task

a35f888

trymzet reviewed Aug 14, 2023

View reviewed changes

tests/integration/tasks/test_luma.py Show resolved Hide resolved

Rafalz13 added 5 commits August 28, 2023 16:08

🚧 Changed command and added dbt_project_path parameter

d4b9079

🔥 Comented source_freshness task

8df4763

🚧 Changed lumaCLI version to 0.0.17

eb30f0a

🐛 Changed version of databricks and lumaCLI

e8cb4d1

✅ Added tests for LumaIngest and CloneRepo tasks

5ae1732

trymzet requested changes Aug 30, 2023

View reviewed changes

Rafalz13 added 5 commits August 30, 2023 14:26

📝 Updated doctrings

8ed1c9e

🎨 Changed luma endpoint to url

0cfa9e3

🐛 Changed parameter name

90c6872

♻️ Changed code structure

395569e

⏪ Revert changes in utils.py

6cb5a99

Rafalz13 requested a review from trymzet August 30, 2023 14:17

trymzet approved these changes Aug 30, 2023

View reviewed changes

Rafalz13 merged commit c900eb1 into dev Aug 30, 2023
3 checks passed

trymzet deleted the dbt_to_viadot_1 branch March 19, 2024 12:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `TransformAndCatalog` flow #720

Add `TransformAndCatalog` flow #720

Rafalz13 commented Jul 14, 2023 •

edited

Loading

angelika233 left a comment

angelika233 Jul 25, 2023

Rafalz13 Aug 9, 2023

angelika233 Jul 25, 2023

angelika233 Jul 25, 2023

Rafalz13 Jul 26, 2023

angelika233 Jul 25, 2023

Rafalz13 Aug 9, 2023

angelika233 Jul 25, 2023

Rafalz13 Jul 26, 2023

angelika233 Jul 25, 2023

Rafalz13 Jul 26, 2023

trymzet commented Aug 2, 2023 •

edited

Loading

Rafalz13 commented Aug 9, 2023

trymzet commented Aug 10, 2023 •

edited

Loading

trymzet commented Aug 11, 2023

trymzet Aug 30, 2023

trymzet left a comment

Add TransformAndCatalog flow #720

Add TransformAndCatalog flow #720

Conversation

Rafalz13 commented Jul 14, 2023 • edited Loading

Summary

Importance

Checklist

angelika233 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

trymzet commented Aug 2, 2023 • edited Loading

Rafalz13 commented Aug 9, 2023

trymzet commented Aug 10, 2023 • edited Loading

trymzet commented Aug 11, 2023

Choose a reason for hiding this comment

trymzet left a comment

Choose a reason for hiding this comment

Add `TransformAndCatalog` flow #720

Add `TransformAndCatalog` flow #720

Rafalz13 commented Jul 14, 2023 •

edited

Loading

trymzet commented Aug 2, 2023 •

edited

Loading

trymzet commented Aug 10, 2023 •

edited

Loading