nautobot / pynautobot

Nautobot Python SDK
https://pynautobot.readthedocs.io/en/latest/index.html
Apache License 2.0
34 stars 29 forks source link

add support for setting a HTTP user agent #171

Closed teunvink closed 2 months ago

teunvink commented 4 months ago

This PR adds support for specifying a HTTP user agent string.

Rationale: in our codebase we try to set user agent strings to the script calling the API. This makes it easier to track down which scripts on which machines use specific APIs, which can become quite difficult to figure out on machines behind NAT and with many different scripts and users on them.

joewesch commented 4 months ago

Setting some headers is also talked about in #158. I don't mind accepting different headers, but I don't want to have to add a param for every header possible. Instead of what you have here, I think accepting a headers param is better and then having the input headers override the default headers.

self.headers = {"Authorization": f"Token {self.token}"}
if headers:
    self.headers.update(**headers)
teunvink commented 4 months ago

@joewesch yes, that makes sense. I hadn't seen that isssue yet, I probably only searched for 'User-Agent'. Should I close this PR, update it with your suggestion?

Adding it just to self.headers won't be enough though (as far as I could see in my tests), you need to add it to the headers in self.http_session.

joewesch commented 4 months ago

Should I close this PR, update it with your suggestion?

No, that was a WIP PR and they haven't updated it in a while. If you get this one updated we can close that one and tell them to use the pattern we set here instead.

Adding it just to self.headers won't be enough though (as far as I could see in my tests), you need to add it to the headers in self.http_session.

That's fine, it was merely a mock up. Add it where it makes sense (or both).

joewesch commented 4 months ago

Let's also update this section of the docs: https://github.com/nautobot/pynautobot/blob/66ebf7f3117b731661510c6dd78f9f80bbbe4291/docs/advanced/session.rst?plain=1#L13-L31

teunvink commented 2 months ago

@joewesch just to be sure, do I need to do anything more with this, or is this just awaiting approval?

joewesch commented 2 months ago

@joewesch just to be sure, do I need to do anything more with this, or is this just awaiting approval?

Yes, I asked that you please update the docs in pynautobot/docs/advanced/session.rst to reflect the changes.

teunvink commented 2 months ago

Ah, I misunderstood that. I've updated the docs, hopefully this is OK.

joewesch commented 2 months ago

Ah, it looks like you may need to rebase from develop again and instead update docs/dev/advanced/session.md as the docs have been migrated to markdown. Sorry for run around.

teunvink commented 2 months ago

Reading this documentation (which I seem to have missed before) makes me realise my PR isn't needed. This works just fine:

nautobot.http_session.headers["User-Agent"] = "my-user-agent"

I'm closing this PR, sorry for the noise.