Skip to content

Comments

Add top-level from_url function#179

Merged
kylebarron merged 21 commits intomainfrom
kyle/top-level-new-store
Jan 31, 2025
Merged

Add top-level from_url function#179
kylebarron merged 21 commits intomainfrom
kyle/top-level-new-store

Conversation

@kylebarron
Copy link
Member

@kylebarron kylebarron commented Jan 30, 2025

Closes #90

Todo:

  • Note: will not read from the environment. Now that we're passing to e.g. S3Store::from_url it should actually read from the environment! (thanks to Always check env when constructing stores, remove from_env #189) (note: we need to test this!!)
  • Add test for reading from the env.
  • Note: Cannot be used for sign
  • Add to docs

We may want to vendor parse_url_opts (which is pretty small), so that we can return the Python classes directly, rather than returning a PrefixStore. At least if we switch to storing a prefix store. #184

@kylebarron kylebarron marked this pull request as ready for review January 31, 2025 00:10
@kylebarron kylebarron marked this pull request as draft January 31, 2025 00:11
@kylebarron
Copy link
Member Author

This PR is waiting on #185, which will enable the prefix handling to work out of the box

@kylebarron
Copy link
Member Author

@maxrjones what do you think about naming this

obstore.store.from_url

?

Then it's also less surprising that it delegates to from_url of each individual store.

@kylebarron kylebarron changed the title Add top-level new_store function Add top-level from_url function Jan 31, 2025
@kylebarron kylebarron marked this pull request as ready for review January 31, 2025 19:09
@maxrjones
Copy link
Member

@maxrjones what do you think about naming this

obstore.store.from_url

?

Then it's also less surprising that it delegates to from_url of each individual store.

I like it!

@kylebarron kylebarron enabled auto-merge (squash) January 31, 2025 21:56
@kylebarron kylebarron merged commit a62b2a6 into main Jan 31, 2025
4 checks passed
@kylebarron kylebarron deleted the kyle/top-level-new-store branch January 31, 2025 21:59
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.

Single store API for all backends, where the url chooses which backend

2 participants