Skip to content
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

using string interpoloation to gather correct pointer for dbt-core te… #80

Merged
merged 29 commits into from
Mar 21, 2022
Merged

using string interpoloation to gather correct pointer for dbt-core te… #80

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
ea1c4b6
Merge `main` into `1.0.latest` (#46)
leahwicz Dec 3, 2021
ffce642
[Backport] Bumping version to 1.0.0 (#47) (#48)
leahwicz Dec 3, 2021
58efd83
Fix package version (#49)
leahwicz Dec 3, 2021
5571085
using string interpoloation to gather correct pointer for dbt-core te…
McKnight-42 Mar 11, 2022
c318e41
created new job for gha to grab correct version of dbt-core to test b…
McKnight-42 Mar 11, 2022
d1a04d0
minor update
McKnight-42 Mar 11, 2022
5a50ae5
Merge branch 'main' into mcknight/fix_gha_for_adapter_releases
McKnight-42 Mar 11, 2022
f21bc6f
adding Get dbt-core-version step to integration.yaml
McKnight-42 Mar 11, 2022
fea3667
Merge branch 'mcknight/fix_gha_for_adapter_releases' of gist.github.c…
McKnight-42 Mar 11, 2022
bac8f96
modifying version parameters
McKnight-42 Mar 14, 2022
7886109
change for integration testing
McKnight-42 Mar 14, 2022
d54ef1d
updating file
McKnight-42 Mar 14, 2022
b6afaa5
readding pull_request_target now that tests pass
McKnight-42 Mar 14, 2022
95af266
make nit: suggested changes
McKnight-42 Mar 18, 2022
fc3163a
testing conditional logic in integration.yml
McKnight-42 Mar 18, 2022
e06d067
updating test names
McKnight-42 Mar 18, 2022
eb12449
creating main.yml versions of new condtional steps for dbt-version ga…
McKnight-42 Mar 18, 2022
64847f0
trying different version of test v.2
McKnight-42 Mar 18, 2022
dade718
v.3 of conditional mix of original version of tests and leah logic
McKnight-42 Mar 18, 2022
b5c38ea
adding comment and changelog entry
McKnight-42 Mar 18, 2022
bfe69d7
changes made after review by @VersusFacit and @Kwigley
McKnight-42 Mar 19, 2022
d19a374
name change
McKnight-42 Mar 19, 2022
31d0b71
minor updates
McKnight-42 Mar 21, 2022
bd7f2ac
updating name of version ref
McKnight-42 Mar 21, 2022
c902a60
name change of dbt-version step to dbt-core-ref to be more descriptiv…
McKnight-42 Mar 21, 2022
ba7531a
Update test_backup_table_option.py
McKnight-42 Mar 21, 2022
f1e3732
Update test_backup_table_option.py
McKnight-42 Mar 21, 2022
698ac86
reseting file that shouldn't of been changed
McKnight-42 Mar 21, 2022
b1b72ba
Merge branch 'mcknight/fix_gha_for_adapter_releases' of gist.github.c…
McKnight-42 Mar 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions .github/workflows/integration.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,21 +151,24 @@ jobs:
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- name: Get dbt-core version
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
# if this is a pull request uses ref of base branch otherwise uses ref of current commit
id: dbt-core-version
run: echo "::set-output name=dbt-version::${{ github.event_name == 'pull_request_target' && github.base_ref || github.ref }}"
- name: Install python dependencies
run: |
pip install --user --upgrade pip
pip install tox
pip --version
tox --version

- name: Install dbt-core latest
- name: Install dbt-core from branch ${{ steps.dbt-core-version.outputs.dbt-version.dbt-core-ref }}
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
run: |
pip install "git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core"
pip install "git+https://github.com/dbt-labs/dbt-core.git@${{ steps.dbt-core-version.outputs.dbt-version }}#egg=dbt-core&subdirectory=core"

- name: Install dbt-postgres latest
- name: Install dbt-postgres from ${{ steps.dbt-core-version.outputs.dbt-version }}
run: |
pip install "git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-postgres&subdirectory=plugins/postgres"
pip install "git+https://github.com/dbt-labs/dbt-core.git@${{ steps.dbt-core-version.outputs.dbt-version }}#egg=dbt-postgres&subdirectory=plugins/postgres"

- name: Run tox (redshift)
if: matrix.adapter == 'redshift'
Expand Down
13 changes: 8 additions & 5 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,21 +93,24 @@ jobs:
uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}

- name: Get dbt-core version
# if this is a pull request uses ref of base branch otherwise uses ref of current commit
id: dbt-core-version
run: echo "::set-output name=dbt-version::${{ github.event_name == 'pull_request' && github.base_ref || github.ref }}"
- name: Install python dependencies
run: |
pip install --user --upgrade pip
pip install tox
pip --version
tox --version

- name: Install dbt-core latest
- name: Install dbt-core from branch ${{ steps.dbt-core-version.outputs.dbt-version.dbt-core-ref }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Install dbt-core from branch ${{ steps.dbt-core-version.outputs.dbt-version.dbt-core-ref }}
- name: Install dbt-core from branch ${{ steps.dbt-core-version.outputs.dbt-version }}

I don't think you need the dbt-core-ref anymore do you?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

name change was based on suggestion by @kwigley in earlier comments #80 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. I think the problem here is the name change only happens in single place (the install step name) but nowhere else. That variable is not set to anything so it is blank.

Also the name change should remove dbt-version then and not just append to it:
Original name: steps.dbt-core-version.outputs.dbt-version
Suggested name: steps.dbt-core-version.outputs.dbt-core-ref
What is here: steps.dbt-core-version.outputs.dbt-version.dbt-core-ref

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Name change modified in steps description, postgres section and steps to all be assigned same.

run: |
pip install "git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core"
pip install "git+https://github.com/dbt-labs/dbt-core.git@${{ steps.dbt-core-version.outputs.dbt-version }}#egg=dbt-core&subdirectory=core"

- name: Install dbt-postgres latest
- name: Install dbt-postgres from ${{ steps.dbt-core-version.outputs.dbt-version }}
run: |
pip install "git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-postgres&subdirectory=plugins/postgres"
pip install "git+https://github.com/dbt-labs/dbt-core.git@${{ steps.dbt-core-version.outputs.dbt-version }}#egg=dbt-postgres&subdirectory=plugins/postgres"

- name: Run tox
run: tox
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@
- Fix table creation statement ordering when including both the BACKUP parameter as well as the dist/sort keys ([#23](https://github.com/dbt-labs/dbt-redshift/issues/60))
- Add unique\_id field to docs generation test catalogs; a follow-on PR to core PR ([#4168](https://github.com/dbt-labs/dbt-core/pull/4618)) and core PR ([#4701](https://github.com/dbt-labs/dbt-core/pull/4701))

### Under the hood
- install compatible branch of dbt-core in unit/integration tests based on merge target ([#80](https://github.com/dbt-labs/dbt-redshift/pull/80))

## dbt-redshift 1.0.0 (December 3, 2021)

## dbt-redshift 1.0.0rc2 (November 24, 2021)

### Under the hood
- Add optional Redshift parameter to create tables with BACKUP NO set, to exclude them from snapshots. ([#18](https://github.com/dbt-labs/dbt-redshift/issues/18), [#42](https://github.com/dbt-labs/dbt-redshift/pull/42))


McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
### Contributors
- [@dlb8685](https://github.com/dlb8685) ([#42](https://github.com/dbt-labs/dbt-redshift/pull/42))

Expand Down
3 changes: 2 additions & 1 deletion tests/integration/backup_table_tests/test_backup_table_option.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ def test__redshift_backup_table_option_project_config_false(self):
# Any view should not contain a BACKUP NO parameter, regardless of the specified config (create will fail)
self.check_backup_param_template('model_backup_true_view', True)


McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
McKnight-42 marked this conversation as resolved.
Show resolved Hide resolved
class TestBackupTableOptionOrder(DBTIntegrationTest):
@property
def schema(self):
Expand Down Expand Up @@ -130,4 +131,4 @@ def test__redshift_backup_table_option_project_config_false(self):
self.check_backup_param_template('model_backup_param_before_distkey', False)

# model_backup_param_before_sortkey should contain a BACKUP NO parameter which precedes a SORTKEY in the table ddl
self.check_backup_param_template('model_backup_param_before_sortkey', False)
self.check_backup_param_template('model_backup_param_before_sortkey', False)