Skip to content

Comments

feature/add dlthub sample#32

Open
oliverw1 wants to merge 57 commits intomainfrom
feature/add-dlthub-sample
Open

feature/add dlthub sample#32
oliverw1 wants to merge 57 commits intomainfrom
feature/add-dlthub-sample

Conversation

@oliverw1
Copy link

@oliverw1 oliverw1 commented Apr 25, 2025

This PR adds a demo project that migrates some tables from an Oracle db to Snowflake using dlthub.

To setup the Oracle DB (on Amazon RDS) and configure the various roles for Snowflake, one can use Terraform in the infra folder.

uv is used for Python dependency and venv management.

The dlthub command is triggered from an OCI container that is itself launched by Conveyor's Airflow ConveyorContainerOperator.


Little aside: this is the commit template from GitHub for this repo. I can understand the first point, but don't understand the rationale for the other 3. Could this be elaborated here and in the root README? The third "rule" (samples_ prefix) doesn't seem to be consistently applied.

  • Updated the root README.md and sample README.md
  • Using the conveyor-samples aws role
  • Airflow dags are prefixed with samples_
  • No .conveyor folder checked-in by mistake

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

General questions on the folder structure:

  • why do we have a .dlt (symlink?) in this folder?
  • we use src/dlt_example, why not top-level structure as dlt does with dlt init?


COPY --from=ghcr.io/astral-sh/uv:0.6.14 /uv /bin/uv

RUN set -eux; \

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need these?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be removed. I've added them out of habit, to ensure bash behaves more like the languages we're used to and to error out on the first failed command (e-flag). Since none of the 3 commands here is likely to give an error, it can (and will) be removed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants