Explicit API key option on client #699
Conversation
| } | ||
|
|
||
| impl ClientHeaders { | ||
| fn apply_to_metadata(&self, metadata: &mut MetadataMap) { |
There was a problem hiding this comment.
I considered doing some of this work on set rather than apply (md key/val creation, concat w/ "Bearer "), but it didn't seem too burdensome to do it on the call
There was a problem hiding this comment.
This makes more sense to me anyway I think. Sure, it's "extra" work but totally irrelevant to performance and this is a bit more flexible
| } | ||
|
|
||
| impl ClientHeaders { | ||
| fn apply_to_metadata(&self, metadata: &mut MetadataMap) { |
There was a problem hiding this comment.
This makes more sense to me anyway I think. Sure, it's "extra" work but totally irrelevant to performance and this is a bit more flexible
|
Just realized my build/test didn't catch all uses of the call, checking integ_tests and such and will push fixes... |
| pub fn set_headers(&self, headers: HashMap<String, String>) { | ||
| let mut guard = self.headers.write(); | ||
| *guard = headers; | ||
| self.headers.write().user_headers = headers; | ||
| } | ||
|
|
||
| /// Set API key, overwriting previous | ||
| pub fn set_api_key(&self, api_key: Option<String>) { | ||
| self.headers.write().api_key = api_key; | ||
| } |
There was a problem hiding this comment.
How do you keep consistent the headers in options.headers and the headers field in ConfiguredClient? Or are we assuming options are just the initial frozen options?
There was a problem hiding this comment.
Options are just the initial frozen options as they always have been (but lang can make theirs mutable if they really want, which Python does but .NET doesn't)
| options: Arc<ClientOptions>, | ||
| headers: Arc<RwLock<HashMap<String, String>>>, | ||
| headers: Arc<RwLock<ClientHeaders>>, |
There was a problem hiding this comment.
I'm working on dynamic endpoints, updating the options field and reconnecting. Is the idea that update headers, that do not need reconnect, with always be updated with set_headers() and not by modifying options?
There was a problem hiding this comment.
Yes, that idea has already been the case (set_headers has already existed, this just moves things around a bit). I think we need to differentiate things that don't require reconnect (i.e. HTTP headers) from things that do (i.e. hostname and TLS options).
3 participants
What was changed
headersfromconnectto theClientOptions(💥 - intentional compatibility break)api_keytoClientOptionsandset_api_keyon client to use as an "Authorization" bearer header if that header not already setChecklist