Add support for authentication in inventory file URLs #710
Conversation
pawamoy
left a comment
There was a problem hiding this comment.
Thanks a lot! Clean code 👍 A few questions.
3c8a294 to
5b8be89
Compare
This allows users to include more complicated username/password strings in the URL
pawamoy
left a comment
There was a problem hiding this comment.
Thanks for the updates!
Just a round on docstrings and comments. I'll apply in batch. Next I'll suggest a few changes to log messages, to make them more informative.
pawamoy
left a comment
There was a problem hiding this comment.
OK WDYT of this? We mention the actual URLs in log messages, to make them more explicit (and/or actionable).
We use %-formatting instead of f-strings for performance (and consistency) reasons.
Sure, we can do that.
I'm happy to change it to %-formatting for consistency, but as far as I know, f-strings are faster 🙂 Edit: Sorry, consistency with what exactly? I can't find any %-formatted strings in the rest of the package, but a lot of f-strings? 🤔 |
f-strings are faster. But when logging debug messages, if the debug level is not actually enabled, you can skip building the string entirely, which logging supports, but only with %-formatting: log.debug(f"Debug message with {costly_string_transform}")
log.debug("String transform of %s not performed if logging level is INFO", costly_string_transform)It's generally not a perf gain, but for the few times it is, we want the consistency of using %-formatting for every logging message 😄 |
Oh yeah, I've only started recently using %-formatting again for log messages, I suppose I didn't update mkdocstrings yet. Will do in another PR 😄 Therefore, feel free to use f-strings here, or %-formatting, it's not important. |
Sorry, I didn't notice you were using the logging module to format the message and not |
|
Released in v0.27.0. |
Thank you for the nice feedback 🙂 |
Implement basic and bearer token authentication for downloading private inventory files in mkdocstrings. This allows users to provide credentials via environment variables in the URL.
Update documentation to reflect these changes and add test cases to ensure functionality.
Closes #707.