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

Limit dbt-tests-adapter build to only produce a wheel #117

Closed
wants to merge 7 commits into from

Conversation

mikealfare
Copy link
Contributor

@mikealfare mikealfare commented Feb 29, 2024

Problem

We can't currently build an source distribution for dbt-tests-adapter because of interactions between hatch, pyproject.toml, and the fact that this is a monorepo. There are also multiple places in the workflow where dbt-tests-adapter is not accounted for (e.g. different working directory, lack of hatch commands, etc.).

Solution

  • Update build steps to only build a wheel
  • Update the check steps to only validate a wheel
  • Update workflows to add option for running tests
  • Update workflows to allow for a working directory to move to ./dbt-tests-adapter
  • Remove duplicated set of steps from the build workflow

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

@mikealfare mikealfare self-assigned this Feb 29, 2024
Copy link

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

…flows to account for building dbt-tests-adapter
@mikealfare mikealfare changed the title Fix sdist build Limit dbt-tests-adapter build to only produce a wheel Feb 29, 2024
@@ -59,19 +59,8 @@ jobs:
uses: ./.github/actions/build-hatch
with:
working-dir: "./dbt-tests-adapter/"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This whole set of steps were duplicated for some reason.

uses: ./.github/actions/build-hatch
with:
working-dir: "./dbt-tests-adapter/"
build-command: "hatch build -t wheel"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Override the default working directory to cd into the dbt-tests-adapter build directory.

@@ -73,18 +73,32 @@ defaults:

jobs:
bump-version-generate-changelog:
name: "Bump package version, Generate changelog"
name: "Bump package version, Generate changelog - dbt-adapters"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Duplicate step to provide different configuration for dbt-adapters and dbt-tests-adapter. dbt-tests-adapter has no tests, and hatch needs to be in dbt-tests-adapter to reference the correct pyproject.toml and associated commands.

archive-name: ${{ inputs.version_number }}-${{ inputs.package }}-${{ inputs.deploy-to }}
working-dir: "./dbt-tests-adapter/"
build-command: "hatch build -t wheel"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Override the default working directory to cd into the dbt-tests-adapter build directory.

@@ -53,6 +53,9 @@ on:
type: string
required: false
default: ''
run-unit-tests:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allow for optional unit tests; dbt-tests-adapter has no tests. This should also probably be updated to something like unit-test-command (which could be empty), to follow suit with build-command and check-command.

@@ -61,6 +64,11 @@ on:
type: string
required: false
default: main
working-dir:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Allow for working directory to be passed in so that hatch can reference the correct pyproject.toml.

@@ -31,14 +31,6 @@ dependencies = [
"dbt-adapters",
"pyyaml",
]

[project.optional-dependencies]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was moved down into the hatch environment dependencies elsewhere. This aligns with that decision.

@@ -50,6 +42,9 @@ Changelog = "https://github.com/dbt-labs/dbt-adapters/blob/main/CHANGELOG.md"
requires = ["hatchling"]
build-backend = "hatchling.build"

[tool.hatch.version]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This got moved from troubleshooting, but it's position now aligns with the root pyproject.toml, so it feels right to keep it here.

uses: dbt-labs/dbt-adapters/.github/workflows/release_prep_hatch.yml@main
name: "Bump package version, Generate changelog - dbt-adapters"
if: ${{ inputs.package == 'dbt-adapters' }}
uses: dbt-labs/dbt-adapters/.github/workflows/release_prep_hatch.yml@fix-sdist-build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Point back to main before merging.

bump-version-generate-changelog-dbt-tests-adapter:
name: "Bump package version, Generate changelog - dbt-tests-adapter"
if: ${{ inputs.package == 'dbt-tests-adapter' }}
uses: dbt-labs/dbt-adapters/.github/workflows/release_prep_hatch.yml@fix-sdist-build
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Point back to main before merging.

@mikealfare
Copy link
Contributor Author

This was resolved by #118

@mikealfare mikealfare closed this Mar 2, 2024
@mikealfare mikealfare deleted the fix-sdist-build branch March 2, 2024 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant