-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add Redshift internal build process workflow #732
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - mostly some questions/changes for the inputs.
invoke-reusable-workflow: | ||
name: Build and Release Internally | ||
|
||
uses: VersusFacit/dbt-release/.github/workflows/internal-archive-release.yml@main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noting this needs to be updated once the dbt-release PR gets merged.
uses: VersusFacit/dbt-release/.github/workflows/internal-archive-release.yml@main | |
uses: dbt-labs/dbt-release/.github/workflows/internal-archive-release.yml@main |
@@ -50,7 +61,7 @@ def _plugin_version() -> str: | |||
install_requires=[ | |||
"dbt-common>=0.1.0a1,<2.0", | |||
"dbt-adapters>=0.1.0a1,<2.0", | |||
f"dbt-postgres~={_plugin_version()}", | |||
f"dbt-postgres~={_plugin_version_trim()}", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikealfare can this be done without ~=
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would need to look at it, but I would think so. We would need to update the function to return a tuple, something along the lines of "dbt-postgres>={},<{}".format(_plugin_version())
where _plugin_version() == (1.8.0b1, 1.9)
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving as is for now with the understanding that more PRs will follow on after testing since none of this can be tested before the workflow gets merged. Since this is a workflow that will be manually submitted there is little risk in this approach.
Tests are failing here due to the |
resolves #38 in epic
Problem
We need a workflow to release things internally
Solution
Write the workflow. I'll be opening up a PR for the workflow this one calls.
Followup actions including reconfiguring secrets and doing a full run start to finish!
Checklist