Migrate Off Circle CI / To Github Actions + dagger.io #923

colin-rogers-dbt · 2023-10-18T23:55:34Z

resolves #719
docs dbt-labs/docs.getdbt.com/#

Problem

Solution

Checklist

I have read the contributing guide and understand what's expected of me
I have run this code in development and it appears to resolve the stated issue
This PR includes tests, or tests are not required/relevant for this PR
This PR has no interface changes (e.g. macros, cli, logs, json artifacts, config files, adapter interface, etc) or this PR has already received feedback and approval from Product or DX

colin-rogers-dbt · 2024-01-09T03:54:58Z

Successful run of integration tests: https://github.com/dbt-labs/dbt-spark/actions/runs/7455834000/job/20285527332

colin-rogers-dbt · 2024-01-09T03:56:20Z

dev-requirements.txt

@@ -1,7 +1,7 @@
 # install latest changes in dbt-core
 # TODO: how to automate switching from develop to version branches?
-git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core


pinning as main is broken until we can migrate

emmyoop

This looks awesome and is so exciting.

.github/workflows/integration.yml

emmyoop · 2024-01-09T16:41:59Z

.github/workflows/integration.yml

+  pull_request_target:
+  workflow_dispatch:
+    inputs:
+      dbt-core-branch:


❤️ ❤️ ❤️

When we get there are we going to also add inputs for dbt-adapter and dbt-common?

good call out when we update all the adapters to use those packages we should update these to include them

emmyoop · 2024-01-09T16:48:02Z

.github/workflows/integration.yml

+    if: >-
+      github.event_name != 'pull_request' ||
+      github.event.pull_request.head.repo.full_name == github.repository ||
+      contains(github.event.pull_request.labels.*.name, 'ok to test')


When is the label used? GitHub automatically doesn't run PRs from forks now. Is this leftover from CircleCI?

emmyoop · 2024-01-09T16:54:47Z

.github/workflows/integration.yml

+    steps:
+      - name: Check out the repository (non-PR)
+        if: github.event_name != 'pull_request_target'
+        uses: actions/checkout@v3
+        with:
+          persist-credentials: false
+
+      - name: Check out the repository (PR)
+        if: github.event_name == 'pull_request_target'
+        uses: actions/checkout@v3
+        with:
+          persist-credentials: false
+          ref: ${{ github.event.pull_request.head.sha }}
+
+      - name: Check if relevant files changed
+        if: github.event_name == 'pull_request_target'
+        # https://github.com/marketplace/actions/paths-changes-filter
+        # For each filter, it sets output variable named by the filter to the text:
+        #  'true' - if any of changed files matches any of filter rules
+        #  'false' - if none of changed files matches any of filter rules
+        # also, returns:
+        #  `changes` - JSON array with names of all filters matching any of the changed files
+        uses: dorny/paths-filter@v2
+        id: get-changes
+        with:
+          token: ${{ secrets.GITHUB_TOKEN }}
+          filters: |
+            spark:
+              - 'dbt/**'
+              - 'tests/**'
+              - 'dev-requirements.txt'


We don't seem to use the output from this job anywhere? We should also be able to accomplish the same thing via filters.

on: pull_request_target: types: - opened branches: - 'releases/**' paths: - '**.js' paths-ignore: - '.changes/*' `

emmyoop · 2024-01-09T16:56:04Z

.github/workflows/integration.yml

+      - name: Check out the repository (PR)
+        if: github.event_name == 'pull_request_target'
+        uses: actions/checkout@v3
+        with:
+          persist-credentials: false
+          ref: ${{ github.event.pull_request.head.sha }}
+


This defeats the purpose of using the pull_request_target trigger.

can you expand on this? Looks like this is what all our other integration test workflows do

Read up a bit more adn this does not defeat the purpose of pull_request_target` because we're already running the workflow from the head which is actually the point.

I understand this is how it's done in the other adapter repos but I'm not sure it's necessary. From the actions/checkout docs

repository: '' # The branch, tag or SHA to checkout. When checking out the repository that # triggered a workflow, this defaults to the reference or SHA for that event. # Otherwise, uses the default branch.

So this feels like a over-complication of checkout.

Discussed offline and we will create a ticket to explain the logic here but leave it as is to follow patterns in adapter repos.

emmyoop · 2024-01-09T17:08:02Z

dagger/run_dbt_spark_tests.py

+
+        elif test_profile == "spark_session":
+            tst_container = tst_container.with_exec(["pip", "install", "pyspark"])
+            tst_container = tst_container.with_exec(["apt-get", "install", "openjdk-17-jre", "-y"])


emmyoop · 2024-01-09T17:09:11Z

dagger/scripts/configure_odbc.sh

+echo "[Simba]\nDriver = /opt/simba/spark/lib/64/libsparkodbc_sb64.so" >> /etc/odbcinst.ini
+dpkg -l | grep Simba # confirm that the driver is installed
+export ODBC_DRIVER="/opt/simba/spark/lib/64/libsparkodbc_sb64.so"


Same comment for why this can't be an env var sent in the workflow.

emmyoop · 2024-01-09T17:16:23Z

docker/Dockerfile

@@ -2,8 +2,8 @@ ARG OPENJDK_VERSION=8
 FROM eclipse-temurin:${OPENJDK_VERSION}-jre

 ARG BUILD_DATE
-ARG SPARK_VERSION=3.3.2


Why did this go back 2 minor versions?

I don't remember... in either case no need to touch this now since we're not using it in CI

emmyoop · 2024-01-09T17:19:25Z

tox.ini

I think the datadog env vars need to be defined in here under passenv (example)

good call out, since we're not using tox we don't need passenv but we do need to pass the env vars to the container

.changes/unreleased/Under the Hood-20230929-161218.yaml

Co-authored-by: Emily Rockman <emily.rockman@dbtlabs.com>

…OffCircleCI

colin-rogers-dbt · 2024-01-09T21:17:42Z

dagger/run_dbt_spark_tests.py

+                    if env_name.startswith(("DD_", "DBT_"))}
+
+TESTING_ENV_VARS.update({
+    "ODBC_DRIVER": "/opt/simba/spark/lib/64/libsparkodbc_sb64.so",


this is set here and not in the workflow since it's defined by the configure_odbc.sh script

emmyoop

A bit more needed to get rid of tox. And I lean towards needing some kind of README in the dagger dir since this is a new tool for us.

emmyoop · 2024-01-10T15:33:10Z

.github/workflows/integration.yml

+      - name: Check out the repository (PR)
+        if: github.event_name == 'pull_request_target'
+        uses: actions/checkout@v3
+        with:
+          persist-credentials: false
+          ref: ${{ github.event.pull_request.head.sha }}
+


Read up a bit more adn this does not defeat the purpose of pull_request_target` because we're already running the workflow from the head which is actually the point.

I understand this is how it's done in the other adapter repos but I'm not sure it's necessary. From the actions/checkout docs

repository: '' # The branch, tag or SHA to checkout. When checking out the repository that # triggered a workflow, this defaults to the reference or SHA for that event. # Otherwise, uses the default branch.

So this feels like a over-complication of checkout.

emmyoop · 2024-01-10T15:44:10Z

.github/workflows/integration.yml

+    paths-ignore:
+      - ".changes/**"
+      - ".flake8"
+      - ".gitignore"
+      - "**.md"


❤️ you've inspired me dbt-labs/dbt-core#9355

emmyoop · 2024-01-10T15:45:44Z

.github/workflows/integration.yml

+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v4
+        with:
+          python-version: "3.11"


Should we be tested against all supported python versions?

emmyoop · 2024-01-10T15:49:04Z

tox.ini

The Makefile still references tox.

emmyoop

Woohoo!

JCZuurmond and others added 17 commits September 29, 2023 11:40

Add Github action for integration test

fd6f6f0

Update tox

795e40a

Fetch spark from https link

ff39c5d

Use Spark version 3.1.2

1505fc6

Seperate running Spark session and thrift

44fe33f

Use Spark 3.1.2 and Hadoop 3.2

2655631

Reset tox.ini

915f67e

Remove base pythons in tox.ini

f0ef215

Fix reference to Docker compose file

e8457df

Remove timeout

842466a

Remove artifact steps

0738f2d

Bump Spark and Hadoop versions

277bef1

Reset Spark and Hadoop version

8d5853d

Update comment

919528a

Add changie

15e48fd

Merge branch 'main' into add-github-workflow-for-integration-tests

ab90c4c

add databricks and PR execution protections

31cb05e

cla-bot bot added the cla:yes label Oct 18, 2023

colin-rogers-dbt and others added 12 commits October 18, 2023 16:55

Merge branch 'main' into migrateOffCircleCI

31eceb5

use single quotes

fd54d7f

remove _target suffix

8de8339

add comment to test

e85232f

specify container user as root

fe3300e

formatting

b37e14b

remove python setup for pre-existing container

51511ec

download simba

98607b6

fix curl call

e6ec414

fix curl call

05a2c08

fix curl call

a89ec58

fix curl call

2a18fad

colin-rogers-dbt requested a review from mikealfare January 8, 2024 23:01

colin-rogers-dbt added 2 commits January 8, 2024 19:45

formatting

d398065

update changie

6108d44

colin-rogers-dbt commented Jan 9, 2024

View reviewed changes

emmyoop requested changes Jan 9, 2024

View reviewed changes

colin-rogers-dbt and others added 3 commits January 9, 2024 09:33

Update .changes/unreleased/Under the Hood-20230929-161218.yaml

d472f3b

Co-authored-by: Emily Rockman <emily.rockman@dbtlabs.com>

formatting fixes and simplify env_var handling

ce92bcf

Merge remote-tracking branch 'origin/migrateOffCircleCI' into migrate…

0c4ed9e

…OffCircleCI

colin-rogers-dbt commented Jan 9, 2024

View reviewed changes

colin-rogers-dbt added 4 commits January 9, 2024 13:30

remove tox, update CONTRIBUTING.md and cleanup GHA workflows

56b14bc

remove tox, update CONTRIBUTING.md and cleanup GHA workflows

9849c1c

install test reqs in main.yml

f9a4c58

install test reqs in main.yml

bbe17a8

colin-rogers-dbt changed the title ~~Migrate Off Circle CI~~ Migrate Off Circle CI / To Github Actions + dagger.io Jan 9, 2024

formatting

3f44e96

colin-rogers-dbt requested a review from emmyoop January 9, 2024 23:34

colin-rogers-dbt self-assigned this Jan 9, 2024

emmyoop requested changes Jan 10, 2024

View reviewed changes

colin-rogers-dbt mentioned this pull request Jan 10, 2024

[ADAP-1093] [Feature] Run integration tests against all supported python versions #970

Closed

3 tasks

colin-rogers-dbt added 3 commits January 10, 2024 09:59

remove tox from dev-requirements.txt and Makefile

afd3866

clarify spark crt instantiation

259ebc7

add comments on python-version

a8a7010

colin-rogers-dbt requested a review from emmyoop January 10, 2024 21:05

emmyoop approved these changes Jan 10, 2024

View reviewed changes

McKnight-42 approved these changes Jan 10, 2024

View reviewed changes

colin-rogers-dbt merged commit f9f75e9 into main Jan 10, 2024
11 checks passed

colin-rogers-dbt deleted the migrateOffCircleCI branch January 10, 2024 21:36

This was referenced Aug 9, 2024

Initial Conversion to GitHub Actions #719

Closed

[Feature] Update documentation of integration tests as it relates to CircleCI / GHA #1089

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Off Circle CI / To Github Actions + dagger.io #923

Migrate Off Circle CI / To Github Actions + dagger.io #923

colin-rogers-dbt commented Oct 18, 2023 •

edited by dbeatty10

Loading

colin-rogers-dbt commented Jan 9, 2024

colin-rogers-dbt Jan 9, 2024

emmyoop left a comment

emmyoop Jan 9, 2024

emmyoop Jan 9, 2024

colin-rogers-dbt Jan 9, 2024

emmyoop Jan 9, 2024

emmyoop Jan 9, 2024

emmyoop Jan 9, 2024

colin-rogers-dbt Jan 9, 2024

emmyoop Jan 10, 2024

emmyoop Jan 10, 2024

emmyoop Jan 9, 2024

emmyoop Jan 9, 2024

emmyoop Jan 9, 2024

colin-rogers-dbt Jan 9, 2024

emmyoop Jan 9, 2024

colin-rogers-dbt Jan 9, 2024

colin-rogers-dbt Jan 9, 2024

emmyoop left a comment

emmyoop Jan 10, 2024

emmyoop Jan 10, 2024

emmyoop Jan 10, 2024

emmyoop Jan 10, 2024

emmyoop left a comment

Migrate Off Circle CI / To Github Actions + dagger.io #923

Migrate Off Circle CI / To Github Actions + dagger.io #923

Conversation

colin-rogers-dbt commented Oct 18, 2023 • edited by dbeatty10 Loading

Problem

Solution

Checklist

colin-rogers-dbt commented Jan 9, 2024

Choose a reason for hiding this comment

emmyoop 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

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

emmyoop 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

emmyoop left a comment

Choose a reason for hiding this comment

colin-rogers-dbt commented Oct 18, 2023 •

edited by dbeatty10

Loading