Skip to content

Update to 1.9 #598

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

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Conversation

Benjamin-Knight
Copy link
Contributor

Closes #597.

All the tests appear to pass and I've successfully used the micro batch functionality locally using this version of the adapter. If anyone can think of additional tests that we require I can look at adding them but the Fabric adapter seemed to have the new functionality covered.

…to merge incremental type as the fabric has removed support for this in favour of microbatch.
All changes are implemented by the upstream fabric adapter.

We need to bring in an updated version of dbt-tests-adapter so we are on the same version as the upstream fabric adapater.
This requires a change of imports for the empty tests as they changed in the adapater tests.
…. This is not supported in Fabric so isn't covered by the upstream adapater.

DBT passes in event times using a UTC string that datetime column types cannot parse.

By converting this to a datetimeoffset it can be filtered against datetime, datetime2 and datetimeoffset columns which can exist in SQL Server.
… the usage of the parse function appears to be preventing index usage.
…trategies that are set in the Fabric adapater to add back in merge.

There are no macros in either the Fabric or SQLServer adapater for the merge type, its all handled by the default DBT implementation.
@Benjamin-Knight
Copy link
Contributor Author

I'm not getting the requirements issue locally and looking it any dbt-core 1.9 or higher should satisfy pip so not quite sure what is causing the build issue.

I can add an extra requirement to guide pip into picking the correct dbt-core version unless someone knows exactly what's going wrong on has a better idea of how to resolve?

… Fabric adapter which generate nested CTEs which cannot run on SQL server.
@cody-scott
Copy link
Collaborator

@Benjamin-Knight can you update the setup.py file to unpin

        "dbt-fabric<1.10.0",
        "dbt-core<1.10.0",

and the dev_requirements.txt file

dbt-tests-adapter

I think unpinning those two should pass the test to start

@Benjamin-Knight
Copy link
Contributor Author

Benjamin-Knight commented Apr 2, 2025

Should I add back in a dbt-tests-adapter adapter version so that it doesn't try and install something for a lower version of dbt-core?

Hard to diagnose exactly what the build server is doing when pip installs it correctly locally.

@cody-scott
Copy link
Collaborator

cody-scott commented Apr 10, 2025

hmm, these pip issues are a bit of a puzzle. One other option is to pin everything between 1.9 and 1.10 and see if that resolves it.

Can you give that a go? The tests also pass locally for me as well so it seems like this is the only roadblock now to 1.9. Thanks for the work you've done to get it here too.

@Benjamin-Knight
Copy link
Contributor Author

@cody-scott I'd be tempted to pin the version of dbt-fabric to 1.9.3, I've got concerns they could update a macro to support nested CTEs which would then break this adapter for anyone who installed it after they release that code.

Update the dbt-core reference to be anything less than v2 now that dbt-core versions are uncoupled from adapater versions
@cody-scott
Copy link
Collaborator

I'm not opposed to that either. There is a scheduled job that runs the tests on sundays to check if any fabric merges breaks this adapter. I could make it nightly though if that's better.

@Benjamin-Knight
Copy link
Contributor Author

The DBT documentation states
Assuming you adapter plugin is pinned to a specific minor version of dbt-core (e.g. ~=1.1.0), you can use the same pin for dbt-tests-adapter.
So I'm going to try that in the requirements-dev and see if that resolves it as its trying to install an old version on the build server when its left to choose one.

@Benjamin-Knight
Copy link
Contributor Author

Benjamin-Knight commented Apr 12, 2025

Looking in totally the wrong place, there is nothing wrong with the requirements file, its the testing setup that is incorrect.

We are trying to test against Python 3.8, DBT core does not support this so likely doesn't have the appropriate wheels any longer so Pip fails when trying to install it all now that the versions have creeped higher.

https://docs.getdbt.com/faqs/Core/install-python-compatibility

I'll put in an update to change the workflow to remove those tests.

Ahh, putting them into this pull request was a bit silly as this one cannot pass to get the tests removed.

@Benjamin-Knight
Copy link
Contributor Author

I'm not opposed to that either. There is a scheduled job that runs the tests on Sundays to check if any fabric merges breaks this adapter. I could make it nightly though if that's better.

The issue is that whilst we may detect the issue unless we fix it quickly anyone installing the adapter, or CI/CD process that runs it is going to pull down the updated fabric version and have code that could potentially not work.

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.

Bump to version 1.9 to track Fabric Adapter
2 participants