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.

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

Conversation

McKnight-42
Copy link
Contributor

@McKnight-42 McKnight-42 commented Mar 11, 2022

resolves #77

Description

adds github action itenterpoloation

NOTE: possible issue to fix later on in cases of pushing to none main or latest branches. ex feature branches

Checklist

  • I have signed the CLA
  • 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
  • I have updated the CHANGELOG.md and added information about my change to the "dbt-redshift next" section.

leahwicz and others added 3 commits December 3, 2021 13:54
* Slack message for failed nightly runs (#41)

* Add Redshift parameter to create tables with backup option specified (#42)

* Update impl and adapters to support backup parameter

* Add test files

* Add test files

* Add PR link to Changelog

* Add EOF newlines

* Debug and split test into two separate cases

* Add contributor info

Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>

* Bumping version to 1.0.0rc2 (#45)

* Bumping version to 1.0.0rc2

* Update changelog

Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>
Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>

Co-authored-by: Dan Bryan <dlb8685@gmail.com>
Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>
* Bumping version to 1.0.0 (#47)

Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>

* Update CHANGELOG.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>
@cla-bot cla-bot bot added the cla:yes label Mar 11, 2022
@McKnight-42 McKnight-42 changed the base branch from main to 1.0.latest March 11, 2022 20:26
@McKnight-42 McKnight-42 force-pushed the mcknight/fix_gha_for_adapter_releases branch from 4b7b12f to 5571085 Compare March 11, 2022 20:31
.github/workflows/main.yml Outdated Show resolved Hide resolved
@McKnight-42 McKnight-42 changed the base branch from 1.0.latest to main March 11, 2022 21:44
@McKnight-42 McKnight-42 changed the base branch from main to 1.0.latest March 11, 2022 21:46
@McKnight-42 McKnight-42 marked this pull request as ready for review March 11, 2022 21:55
@McKnight-42 McKnight-42 changed the base branch from 1.0.latest to main March 11, 2022 21:55
@McKnight-42 McKnight-42 changed the base branch from main to 1.0.latest March 11, 2022 21:59
@McKnight-42 McKnight-42 changed the base branch from 1.0.latest to main March 11, 2022 22:04
@McKnight-42 McKnight-42 changed the base branch from main to 1.0.latest March 11, 2022 22:16
@McKnight-42 McKnight-42 changed the base branch from 1.0.latest to main March 11, 2022 22:50
@McKnight-42
Copy link
Contributor Author

Tests pass when we change pull_request_target to pull_request to go around some of our strict gha testing workflows.

@McKnight-42 McKnight-42 self-assigned this Mar 18, 2022
Copy link
Contributor

@VersusFacit VersusFacit left a comment

Choose a reason for hiding this comment

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

couple small comments. Slack me and I'll prioritize a rereview.

CHANGELOG.md Outdated Show resolved Hide resolved
.github/workflows/main.yml Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
- 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.

Copy link
Contributor

@leahwicz leahwicz left a comment

Choose a reason for hiding this comment

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

Looks good! Thanks Matt!

@McKnight-42 McKnight-42 merged commit b400971 into main Mar 21, 2022
@McKnight-42 McKnight-42 deleted the mcknight/fix_gha_for_adapter_releases branch March 21, 2022 21:56
McKnight-42 added a commit that referenced this pull request Mar 21, 2022
McKnight-42 added a commit that referenced this pull request Mar 25, 2022
* backporting #80 to unstick releases for adapters

* updating dev_requirements and commenting out lines in main.yaml

* adding backport of pr#58 to get rid of testing error based on pr #4512 in dbt-core

* uncommenting to see if we can make pointers of local install and tox install the same

* testing if tox change is all thats needed

* tests failed proving the addition is needed as backport #58

* trying out removal of dbt-core-ref install

* remove white-space

* remove whitespace

* remove trailing whitespace on main.yml
abbywh pushed a commit to abbywh/dbt-redshift that referenced this pull request Oct 11, 2023
dbt-labs#80)

* Merge `main` into `1.0.latest` (dbt-labs#46)

* Slack message for failed nightly runs (dbt-labs#41)

* Add Redshift parameter to create tables with backup option specified (dbt-labs#42)

* Update impl and adapters to support backup parameter

* Add test files

* Add test files

* Add PR link to Changelog

* Add EOF newlines

* Debug and split test into two separate cases

* Add contributor info

Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>

* Bumping version to 1.0.0rc2 (dbt-labs#45)

* Bumping version to 1.0.0rc2

* Update changelog

Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>
Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>

Co-authored-by: Dan Bryan <dlb8685@gmail.com>
Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>

* [Backport] Bumping version to 1.0.0 (dbt-labs#47) (dbt-labs#48)

* Bumping version to 1.0.0 (dbt-labs#47)

Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>

* Update CHANGELOG.md

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>

* Fix package version (dbt-labs#49)

* using string interpoloation to gather correct pointer for dbt-core tests against release branches

* created new job for gha to grab correct version of dbt-core to test branch against

* minor update

* adding Get dbt-core-version step to integration.yaml

* modifying version parameters

* change for integration testing

* updating file

* readding pull_request_target now that tests pass

* make nit: suggested changes

* testing conditional logic in integration.yml

* updating test names

* creating main.yml versions of new condtional steps for dbt-version gather

* trying different version of test v.2

* v.3 of conditional mix of original version of tests and leah logic

* adding comment and changelog entry

* changes made after review by @VersusFacit and @kwigley

* name change

* minor updates

* updating name of version ref

* name change of dbt-version step to dbt-core-ref to be more descriptive iof where version is coming from

* Update test_backup_table_option.py

* Update test_backup_table_option.py

* reseting file that shouldn't of been changed

Co-authored-by: leahwicz <60146280+leahwicz@users.noreply.github.com>
Co-authored-by: Dan Bryan <dlb8685@gmail.com>
Co-authored-by: Jeremy Cohen <jeremy@dbtlabs.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Github Build Bot <buildbot@fishtownanalytics.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CT-348] Unit tests broken for release branches
4 participants