Validate require-dbt-version before validating dbt_project.ymls chema #2726
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
update changelog
beckjake
force-pushed
the
fix/require-version-validation
branch
from
4c5ab89
to
1f8e292
Compare
jtcohen6
approved these changes
Aug 25, 2020
gshank
approved these changes
Aug 26, 2020
Co-authored-by: Kyle Wigley <kyle@fishtownanalytics.com>
QMalcolm
added a commit
that referenced
this pull request
May 29, 2024
…est_project We've done two things in this commit, which arguably _should_ have been done in two commits. First we moved the version specifier tests from `test_runtime.py::TestRuntimeConfig` to `test_project.py::TestGetRequiredVersion` this is because what is really being tested is the `_get_required_version` method. Doing it via `RuntimeConfig.from_parts` method made actually testing it a lot harder as it requires setting up more of the world and running with a _full_ project config dict. The second thing we did was convert it from the old unittest implementation to a pytest implementation. This saves us from having to create most of the world as we were doing previously in these tests. Of note, I did not move the test `test_unsupported_version_range_bad_config`. This test is a bit different from the rest of the version specifier tests. It was introduced in [1eb5857](1eb5857) of [#2726](#2726) to resolve [#2638](#2638). The focus of #2726 was to ensure the version specifier checks were run _before_ the validation of the `dbt_project.yml`. Thus what this test is actually testing for is order of operations at parse time. As such, this is really more a _functional_ test than a unit test. In the next commit we'll get this test moved (and renamed)
QMalcolm
added a commit
that referenced
this pull request
May 29, 2024
…est_project We've done two things in this commit, which arguably _should_ have been done in two commits. First we moved the version specifier tests from `test_runtime.py::TestRuntimeConfig` to `test_project.py::TestGetRequiredVersion` this is because what is really being tested is the `_get_required_version` method. Doing it via `RuntimeConfig.from_parts` method made actually testing it a lot harder as it requires setting up more of the world and running with a _full_ project config dict. The second thing we did was convert it from the old unittest implementation to a pytest implementation. This saves us from having to create most of the world as we were doing previously in these tests. Of note, I did not move the test `test_unsupported_version_range_bad_config`. This test is a bit different from the rest of the version specifier tests. It was introduced in [1eb5857](1eb5857) of [#2726](#2726) to resolve [#2638](#2638). The focus of #2726 was to ensure the version specifier checks were run _before_ the validation of the `dbt_project.yml`. Thus what this test is actually testing for is order of operations at parse time. As such, this is really more a _functional_ test than a unit test. In the next commit we'll get this test moved (and renamed)
QMalcolm
added a commit
that referenced
this pull request
May 30, 2024
…est_project We've done two things in this commit, which arguably _should_ have been done in two commits. First we moved the version specifier tests from `test_runtime.py::TestRuntimeConfig` to `test_project.py::TestGetRequiredVersion` this is because what is really being tested is the `_get_required_version` method. Doing it via `RuntimeConfig.from_parts` method made actually testing it a lot harder as it requires setting up more of the world and running with a _full_ project config dict. The second thing we did was convert it from the old unittest implementation to a pytest implementation. This saves us from having to create most of the world as we were doing previously in these tests. Of note, I did not move the test `test_unsupported_version_range_bad_config`. This test is a bit different from the rest of the version specifier tests. It was introduced in [1eb5857](1eb5857) of [#2726](#2726) to resolve [#2638](#2638). The focus of #2726 was to ensure the version specifier checks were run _before_ the validation of the `dbt_project.yml`. Thus what this test is actually testing for is order of operations at parse time. As such, this is really more a _functional_ test than a unit test. In the next commit we'll get this test moved (and renamed)
QMalcolm
added a commit
that referenced
this pull request
May 31, 2024
* Create `runtime_config` fixture and necessary upstream fixtures * Check for better scoped `ProjectContractError` in test_runtime tests Previously in `test_unsupported_version_extra_config` and `test_archive_not_allowed` we were checking for `DbtProjectError`. This worked because `ProjectContractError` is a subclass of `DbtProjectError`. However, if we check for `DbtProjectError` in these tests than, some tangential failure which raises a `DbtProejctError` type error would go undetected. As we plan on modifying these tests to be pytest in the coming commits, we want to ensure that the tests are succeeding for the right reason. * Convert `test_str` of `TestRuntimeConfig` to a pytest test using fixtures * Convert `test_from_parts` of `TestRuntimeConfig` to a pytest test using fixtures While converting `test_from_parts` I noticed the comment > TODO(jeb): Adapters must assert that quoting is populated? This led me to beleive that `quoting` shouldn't be "fully" realized in our project fixture unless we're saying that it's gone through adapter instantiation. Thus I update the `quoting` on our project fixture to be an empty dict. This change affected `test__str__` in `test_project.py` which we thus needed to update accordingly. * Convert runtime version specifier tests to pytest tests and move to test_project We've done two things in this commit, which arguably _should_ have been done in two commits. First we moved the version specifier tests from `test_runtime.py::TestRuntimeConfig` to `test_project.py::TestGetRequiredVersion` this is because what is really being tested is the `_get_required_version` method. Doing it via `RuntimeConfig.from_parts` method made actually testing it a lot harder as it requires setting up more of the world and running with a _full_ project config dict. The second thing we did was convert it from the old unittest implementation to a pytest implementation. This saves us from having to create most of the world as we were doing previously in these tests. Of note, I did not move the test `test_unsupported_version_range_bad_config`. This test is a bit different from the rest of the version specifier tests. It was introduced in [1eb5857](1eb5857) of [#2726](#2726) to resolve [#2638](#2638). The focus of #2726 was to ensure the version specifier checks were run _before_ the validation of the `dbt_project.yml`. Thus what this test is actually testing for is order of operations at parse time. As such, this is really more a _functional_ test than a unit test. In the next commit we'll get this test moved (and renamed) * Create a better test for checking that version checks come before project schema validation * Convert `test_get_metadata` to pytest test * Refactor `test_archive_not_allowed` to functional test We do already have tests that ensure "extra" keys aren't allowed in the dbt_project.yaml. This test is different because it's checking that a specific key, `archive`, isn't allowed. We do this because at one point in time `archive` _was_ an allowed key. Specifically, we stopped allowing `archive` in dbt-core 0.15.0 via commit [f26948d](f26948d). Given that it's been 5 years and a major version, we could probably remove this test, but let's keep it around unless we start supporting `archive` again. * Convert `warn_for_unused_resource_config_paths` tests to use pytest
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #2638
Description
This just moves the check up to before we parse things, and pushes the resolved versions through to the final configuration object.
Checklist
CHANGELOG.mdand added information about my change to the "dbt next" section.