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

Lift + shift for cross-db macros #25

Merged
merged 15 commits into from
Aug 4, 2022
Merged

Lift + shift for cross-db macros #25

merged 15 commits into from
Aug 4, 2022

Conversation

@jtcohen6
Copy link
Collaborator Author

jtcohen6 commented May 18, 2022

Functional tests are running, which is actually amazing. The other tests are running into a dbt-core error while parsing profiles.yml, which I've just opened as dbt-labs/dbt-core#5268

@jtcohen6 jtcohen6 force-pushed the jerco/utils-lift-shift branch from 10a0140 to 8f01640 Compare August 3, 2022 09:53
@jtcohen6 jtcohen6 changed the title Initialize lift + shift for cross-db macros Lift + shift for cross-db macros Aug 3, 2022
@jtcohen6
Copy link
Collaborator Author

jtcohen6 commented Aug 3, 2022

The idea would be:

  • A new minor release, requiring dbt-core v1.2.0, with backwards compatibility for folks calling these macros directly — spark_utils.dateadd. I don't think that's many people, but still!
  • A subsequent minor release that removes all backwards compatibility

@jtcohen6 jtcohen6 marked this pull request as ready for review August 3, 2022 10:41
@jtcohen6 jtcohen6 requested a review from dbeatty10 August 3, 2022 10:41
Copy link
Contributor

@dbeatty10 dbeatty10 left a comment

Choose a reason for hiding this comment

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

You are amazing.

Love the creation of the macros/dbt_utils/cross_db_utils/deprecated folder. Separates things nicely.

Comment on lines +1 to +5
pytest
pyodbc==4.0.32
git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-core&subdirectory=core
git+https://github.com/dbt-labs/dbt-core.git#egg=dbt-tests-adapter&subdirectory=tests/adapter
git+https://github.com/dbt-labs/dbt-spark.git#egg=dbt-spark[ODBC]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is helpful!

Comment on lines +1 to +8
[pytest]
filterwarnings =
ignore:.*'soft_unicode' has been renamed to 'soft_str'*:DeprecationWarning
ignore:unclosed file .*:ResourceWarning
env_files =
test.env
testpaths =
tests/functional
Copy link
Contributor

Choose a reason for hiding this comment

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

Last Week I Learned (LWIL):

Example

Let's say I have an environment variable defined locally:
DBT_DATABASE_HOST=localhost

And test.env contains:
DBT_DATABASE_HOST=somethingelse

By default, pytest-dotenv will keep the former and ignore the latter. Scroll down to see how to configure the opposite behavior.

Override default behavior

So it depending on how we want pytest + pytest-dotenv to behave, we either will want to include the following (or not!):

env_override_existing_values = 1

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A great LWIL!

This makes sense to me, insofar as I might have a test.env, but then also want to do:

export DBT_DATABASE_HOST=localhost
python3 -m pytest ...

I would indeed expect the env var I've just supplied to override the one in test.env

@jtcohen6 jtcohen6 merged commit 65ae910 into main Aug 4, 2022
@jtcohen6 jtcohen6 mentioned this pull request Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants