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

Add support for dbt core 1.4 #345

Closed
wants to merge 9 commits into from

Conversation

l-j
Copy link
Contributor

@l-j l-j commented Apr 13, 2023

Resolves #333.

Checklist:

  • support Python 3.11 (only if your adapter's dependencies allow)
  • Consolidate timestamp functions & macros
  • Replace deprecated exception functions
  • Add support for more tests

Works with my dbt projects using both Azure SQL Database and Microsoft SQL Server 2017.

This PR requires changes around incremental_predicates, but as I do not use incremental materialisations much, I do not have time to test everything properly and write tests, so for now I have marked this feature as not implemented. The changes required do not look huge, so I may try to add them later.

I believe, there are some problems with the CI in this repository, but as you can see all the tests are green here:
https://github.com/l-j/dbt-sqlserver/pull/4/checks

@romiof
Copy link

romiof commented May 4, 2023

Hello guys! Any news about this PR?

@mikaelene
Copy link
Collaborator

Looks like we need a CI-container for Python 3.11. Who knows how to get that in here?

@l-j
Copy link
Contributor Author

l-j commented May 5, 2023

@mikaelene My understanding is that with the current Github Actions configuration, we can only have a container for the changes that are already on the master branch (see: https://github.com/l-j/dbt-sqlserver/blob/master/.github/workflows/publish-docker.yml#L9), so a Python 3.11 container will be built after the merge.

And basically, this is what I did to my fork to make CI work. so maybe we should also build it for pull requests to master? If that is an acceptable solution for now, I can prepare a quick pull request with the change. I cannot see the list of maintainers for this repository. @dataders, can you help with that?

@mikaelene
Copy link
Collaborator

Ahh. Get it now. So basically these test will not pass until we merge this to master. Maybe we should create a PR for just building the containers for 3.11? I am a maintainer (and also the creator), but haven't been very active in a while. If you create a PR for just building more versions of the containers, I can approve. It is always easier to merge a PR with green checkmarks.

I haven't used incremental predicates but it looks like a neat feature. But I guess they are not needed to make this release. Maybe should write something in the docs on this.

@l-j
Copy link
Contributor Author

l-j commented May 5, 2023

@mikaelene I think this will do: 03b490c, but I am not an expert in the Gihub Actions.

Now build process is waiting for approval.

And you are right about the documentation. As far as I can see now, the documentation has been moved to the separate repository, so I can prepare a PR there as well, when that PR is accepted.

@l-j
Copy link
Contributor Author

l-j commented May 8, 2023

@mikaelene I see that the step Publish Docker images failed with the following error message:

ERROR: denied: installation not allowed to Write organization package
Error: buildx failed with: ERROR: denied: installation not allowed to Write organization package

And indeed, according to this page, it can be caused by missing permissions.

If we change the trigger rule to allow pull requests to trigger the publication of Docker images, we will need to make additional changes to the repository configuration as described here.

@dataders dataders added this to the v1.4.0 milestone May 10, 2023
setup.py Outdated Show resolved Hide resolved
@l-j
Copy link
Contributor Author

l-j commented May 12, 2023

After modifying setup.py, we get the following:

The conflict is caused by:
    dbt-tests-adapter 1.4.1 depends on dbt-core==1.4.1
    dbt-sqlserver 1.4.0 depends on dbt-core~=1.4.5

so I loosen the dev requirements and pin dbt-tests-adapter to the dbt-core version.

@dataders
Copy link
Collaborator

closing because your commits were merged via #350. Thanks @l-j for getting the ball rolling on this btw!

@dataders dataders closed this May 12, 2023
@sdebruyn sdebruyn mentioned this pull request May 12, 2023
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.

upgrade to support dbt-core v1.4.0
4 participants