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

[dagster-deltalake] Fix GcsConfig() ImportError and TypeError for partitioned assets #22054

Conversation

thmswt
Copy link
Contributor

@thmswt thmswt commented May 23, 2024

Summary & Motivation

Closes [dagster-deltalake] GcsConfig ImportError and TypeError for partitioned assets #22053

How I Tested These Changes

python -m pytest python_modules/libraries/dagster-deltalake/dagster_deltalake_tests

@cmpadden
Copy link
Contributor

Hi @ion-elgreco - I see that you last worked on this section of code, and wanted to get your input. Does this look good to you?

@cmpadden cmpadden self-assigned this May 31, 2024
@ion-elgreco
Copy link
Contributor

@cmpadden the config one is fine, but changing the expression param in the filter seems to not solve anything. Pyarrow11+ supports the filter method, and they all use expression as a parameter.

But I would say these are the least of the issues, there are many issues and missing implementations in the current io manager.

I tried upstreaming this a while ago but there was no traction, so I now internally maintain the improved versions

@cmpadden
Copy link
Contributor

Thanks for the insight, @ion-elgreco ; sorry to hear that getting it upstream has been difficult.

@thmswt the build kite tests pass, however it looks like ruff needs to be run to format/sort the imports. Would you be able to do that when you get the chance, please?

@thmswt
Copy link
Contributor Author

thmswt commented Jun 3, 2024

@cmpadden imports are sorted now as suggested by ruff.

@ion-elgreco thanks for your input. Tested it with Pyarrow12+. Version 16.1.0 works with expression param though.

@cmpadden cmpadden merged commit 8029bc5 into dagster-io:master Jun 3, 2024
1 check passed
danielgafni pushed a commit to danielgafni/dagster that referenced this pull request Jun 18, 2024
…titioned assets (dagster-io#22054)

## Summary & Motivation

Closes [[dagster-deltalake] GcsConfig ImportError and TypeError for
partitioned assets
dagster-io#22053](dagster-io#22053)

## How I Tested These Changes

`python -m pytest
python_modules/libraries/dagster-deltalake/dagster_deltalake_tests`
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.

[dagster-deltalake] GcsConfig ImportError and TypeError for partitioned assets
3 participants