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

Update dagster_sdf to be compatible with recent SDF versions (0.3.23 -> 0.10.8) #26665

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

elliottower
Copy link

Summary & Motivation

The current SDF version is 0.10.8, but the dagster_sdf package requires a significantly older version 0.3.23.

How I Tested These Changes

I modified the library locally on my machine and ran the official dagster sdf tutorial from the SDF docs (link)

Following this tutorial with the older version worked as well, but limited ability to do advanced SDF features (e.g., BigQuery integration). The documentation is also quite different from so many version changes, so it was hard to figure out the issue.

Changelog

Insert changelog entry or delete this section.

Copy link
Contributor

@mlarose mlarose left a comment

Choose a reason for hiding this comment

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

Hi @elliottower, thanks for the contribution.

What are the reasons to keep such a narrow gap and force the upgrade for users with older SDF?

@elliottower
Copy link
Author

Hi @elliottower, thanks for the contribution.

What are the reasons to keep such a narrow gap and force the upgrade for users with older SDF?

When I was using it it basically forced me to use an older version, and wouldn't let me use a new version, but I can see having that for backwards compatibility makes sense. The existing code has 0.3.23 - 0.3.24, maybe it could just be loosened to be between 0.3.23 and 0.10.9 (most recent version).

Feel free to close this and do your own PR with different upper bounds, and I'm sure there's various tests that have to be done to ensure things work with the new version, all I was basically advocating for is that it at least have the newest version be in the upper bound, the lower bound I can see keeping the same as before.

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.

Update dagster-sdf to most recent versions of SDF (0.10.8 vs 0.3.24)
2 participants