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

Refactor azure storage with crate updates #644

Merged
merged 10 commits into from
Jun 17, 2022
Merged

Conversation

roeap
Copy link
Collaborator

@roeap roeap commented Jun 16, 2022

Description

With the latest version of the azure SDKs some annoyances got fixed. specifically, list now returns something Sendable, which allows us to significantly simplify the list_objs method. Secondly the errors are no longer a deeply nested nightmare 😆.

Also added a test for validating that service principal access works (#609) - currently i am guessing the issues encountered there are more related to #643

Related Issue(s)

validate the SP access works for #609

Documentation

@Blajda
Copy link
Collaborator

Blajda commented Jun 17, 2022

Hi @roeap you can remove tokio-util from the dependency list since it was introduced for the !Send issue.

@roeap
Copy link
Collaborator Author

roeap commented Jun 17, 2022

Thanks @Blajda - always happy to be able to slim down dependencies 😆

wjones127
wjones127 previously approved these changes Jun 17, 2022
Copy link
Collaborator

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

This is a great cleanup! LGTM.

@roeap
Copy link
Collaborator Author

roeap commented Jun 17, 2022

@wjones127 - we saw failing CI due to windows runners running out of disk space. Based on pola-rs/polars#3122 (comment) I switched to using Swatinem/rust-cache, which seems to do some nice things around managing cache size and utilization.

Right now builds are passing again, but this could just be related to using a "fresh" cache, we'll have to keep an eye on it.

Could you have another look?

@roeap roeap enabled auto-merge (squash) June 17, 2022 08:47
Copy link
Collaborator

@wjones127 wjones127 left a comment

Choose a reason for hiding this comment

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

This looks like it helped Polars and they are still using it, so should be good. I'll keep on eye on that CI job.

@roeap roeap merged commit cd90d77 into delta-io:main Jun 17, 2022
@roeap roeap deleted the azure-update branch June 17, 2022 11:52
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.

3 participants