Azure / azure-sdk-for-rust

This repository is for active development of the *unofficial* Azure SDK for Rust. This repository is *not* supported by the Azure SDK team.
MIT License
685 stars 232 forks source link

Consider Into<dyn TokenCredential> instead of Arc<> for creating clients #1571

Open heaths opened 6 months ago

heaths commented 6 months ago

Guarding access to TokenCredential in its entirety seems overkill. Credentials really only need to guard against multiple writes, which should be rare and, in many cases, inconsequential if there are multiple writers (just wasted time getting multiple tokens e.g., bearer tokens).

Instead of guarding access to the entire TokenCredential, should we just require an Rc<dyn TokenCredential> instead?

demoray commented 6 months ago

Arc is a thread safe Rc. This is important in the case of threaded async runtimes.

The underlying token cache is stored in a RwLock, so in most cases, the impact should be negligible.

heaths commented 6 months ago

Changing the title a bit after the meeting @demoray, @cataggar, and I had today. I appreciate now why we pass an Arc<T> - because it implements Send needed for tokio - but I'm wondering, because it complicates (makes much more verbose) samples, whether we could have clients accept an Into<dyn TokenCredential> since Arc<T> has a blanket implementation of Into<T> that is more versatile. Clients could then wrap it themselves in an Arc<T>.