-
Notifications
You must be signed in to change notification settings - Fork 11
feat(pins): add support for custom storage options in board_s3
#237
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
Conversation
- Introduce the ability to pass arbitrary storage options to the underlying fsspec S3FileSystem in `board_s3`. - This enhancement allows for greater flexibility when connecting to S3-compatible services by enabling the specification of custom credentials, endpoints, and other S3FileSystem parameters. - Added documentation and examples to illustrate how to use the new `storage_options` parameter, including an example for connecting to Backblaze B2. This change enables users to more easily integrate with a variety of S3-compatible storage solutions, improving the library's versatility and user experience.
- Replace `**kwargs` with `**storage_options` to accurately reflect the intended parameter in `board_s3` function, ensuring the correct handling of storage options.
- This commit adds an import statement for the `pins` module in the docstring example of the `board_s3` function. This change ensures that the example is self-contained and can be executed without prior context, improving the documentation's usability for new users.
- Adjusted the function definition of `board_s3` to span multiple lines, improving code readability and maintainability. - Ensured consistency with project coding standards for function definitions.
|
@isabelizimm @machow I think I'm running into an issue with the docs, but I'm not quite sure how to address the issue, as it uses quartodoc (which I'm happy to learn, but may need help getting up-to-speed on). In case it's helpful, this is a direct link to the exact line that is failing. |
|
Ah thank you so much for putting up this PR! Let me peek at the docs failures and run the tests on other backends to see if something jumps out 👀 update: These tests + docs failures should be fixed in #242; needed a bit of maintenance for the latest fsspec version that reordered some protocol names 👍 |
|
One small nit, but everything else looks great! 😄 |
|
Woot! Looks like all of the non-skipped stuff passed 😄. |
- Implemented a warning for users setting `listings_expiry_time` to a non-zero value in `board_s3` function to alert them about potential unexpected cache behaviour. - Refactored the handling of `storage_options` to ensure `listings_expiry_time` is explicitly set, either by the user or to a default of 0, to improve clarity and maintainability of the code. This change aims to guide users towards optimal performance settings and enhance the reliability of cache operations with S3 boards.
|
Okie dokes, just pushed the requested change, @isabelizimm! |
isabelizimm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Spectacular! This is so great to expand the options for S3 pins users 🔥
board_s3.storage_optionsparameter, including an example for connecting to Backblaze B2.This change enables users to more easily integrate with a variety of S3-compatible storage solutions, improving the library's versatility and user experience.
Address issue: #235
Tag @isabelizimm!