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

Fix Azure storage for assets and remote storage #93

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jtnicholl-cosairus
Copy link

@jtnicholl-cosairus jtnicholl-cosairus commented Jun 21, 2024

Though the documentation claims you can use any protocol that Smart Open supports, Azure storage accounts don't work. I wrote a quick fix to get it working.

Description

Smart Open allows providing transport parameters, which are optional, unless you're using Azure storage containers, then they are required and must include a client that has the connection string.

I made a quick fix to grab the connection string from the environment variable AZURE_STORAGE_CONNECTION_STRING if the URL is Azure. I know I probably didn't follow conventions correctly, but I needed to use Azure and that got it working.

Types of change

Small change to remote.py.

Checklist

  • I confirm that I have the right to submit this contribution under the project's MIT license.
  • I ran the test suite, and all new and existing tests passed.
  • My changes don't require a change to the documentation, or if they do, I've added all required information.

@jtnicholl-cosairus
Copy link
Author

So after this I moved on to remote storage with push and pull, and ran into another problem.
Smart Open expects Azure URLs to start with azure://, but Cloudpathlib expects az://. So when Cloudpathlib sees azure:// it thinks it's a local file path and changes it to azure:/, with the added bonus on Windows that it becomes azure:\ instead (along with all the other slashes). This prevents Azure from working for remote storage even after the above fix.
I pushed another commit to replace az:// with azure:// in remote.py before passing it to Smart Open, which feels a bit hacky but I don't know how else to get it working.

@jtnicholl-cosairus jtnicholl-cosairus changed the title Fix Azure storage for assets Fix Azure storage for assets and remote storage Jun 28, 2024
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.

1 participant