search

yoshidan / google-cloud-rust

Google Cloud Client Libraries for Rust.
MIT License
233 stars 85 forks source link

[Storage] remove default implementation for SignedUrlOptions #127

Closed open-schnick closed 1 year ago

open-schnick commented 1 year ago

Hi :)

My Problem

When trying to create a signed url for cloud-storage I realized that using the Default implementation of the SignedUrlOptions does not work. Looking at the code that is quite obvious as there cannot be a sensible default for things like credentials. Imo its not really user-friendly to provide defaults that do not work. I get it, that it is convenient to use default in Tests but as a user of the crate its not cool.

My proposed solution

I renamed SignedUrlOptions -> SignedUrlConfig and added SignedUrlConfigOptions to represent some config options that are not required and thus can implement default.

I am open for discussion and feedback is greatly appreciated :)

open-schnick commented 1 year ago

Also we should think about bumping up the crate version even further. As the refactoring implies a breaking api change we should use a major version aka 1.0.0

open-schnick commented 1 year ago

Should work now @yoshidan

open-schnick commented 1 year ago

Honestly i dont have any idea how to fix that cargo deny error. Any ideas?

yoshidan commented 1 year ago

Honestly i dont have any idea how to fix that cargo deny error. Any ideas?

I fixed cargo deny error by this commit. If you rebase or merge main branch it, it will work. https://github.com/yoshidan/google-cloud-rust/commit/1265e71126df74e5d8809d5768b15d7e17b9f08c

We will merge it if the following discussion is settled. https://github.com/yoshidan/google-cloud-rust/pull/127#discussion_r1157916809