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

Support dbt 1.9 #143

Merged
merged 14 commits into from
Feb 9, 2025
Merged

Conversation

millin
Copy link
Contributor

@millin millin commented Feb 5, 2025

No description provided.

@millin

This comment was marked as resolved.

@millin millin marked this pull request as ready for review February 5, 2025 09:19
@millin millin force-pushed the update_suppoted_versions branch 5 times, most recently from 3070790 to a1ae809 Compare February 5, 2025 11:19
@millin millin force-pushed the update_suppoted_versions branch from a1ae809 to 364cc97 Compare February 5, 2025 14:06
@tomasfarias
Copy link
Owner

I see some issues with the documentation build, I think related to the pyproject.toml dependencies not updated in line with docs/requirements.txt. The readthedocs build is using the pyproject.toml, essentially running poetry install -E git -E s3 --with docs, so I think we should bump those dependencies too.

Happy to take a look later today and try to get this fixed. Everything else looks good to me.

@tomasfarias
Copy link
Owner

tomasfarias commented Feb 6, 2025

@millin Hope you don't mind I've pushed a few commits to clear the remaining CI errors.

Essentially, seems like something changed in the readthedocs builds and poetry and readthedocs are now installing/expecting dependencies in different virtualenvs. I ended up solving the issue with the poetry export plugin, and outputting a docs/requirements.txt file during the build process that readthedocs can pick up during install, and everything builds correctly now.

I've also removed the committed docs/requirements.txt for this reason.

Moreover, Snyk was reporting a vulnerability on zipp < 3.19.1. I've pinned zipp >= 3.19.1, but the alert didn't clear. Anyways, with the removal of docs/requirements.txt, snyk is not reporting anything. The usefulness of this tool is really in question.

All that being said, I think CI should build successfully now. I'll take a closer look at this PR tomorrow or on Friday and get it merged+released then.

Amazing work!

@millin
Copy link
Contributor Author

millin commented Feb 6, 2025

I'm glad you were able to correct CI errors 👍

@tomasfarias
Copy link
Owner

I don't see anything to comment here, work looks great! With CI issues resolved, let's ship this.

@tomasfarias tomasfarias merged commit 2956a28 into tomasfarias:master Feb 9, 2025
25 of 48 checks passed
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