Skip to content

Enhanced User-Agent - #88

Merged
shiyuechengineer merged 4 commits into
meraki:masterfrom
petecj2:master
May 6, 2020
Merged

Enhanced User-Agent#88
shiyuechengineer merged 4 commits into
meraki:masterfrom
petecj2:master

Conversation

@petecj2

@petecj2 petecj2 commented Apr 29, 2020

Copy link
Copy Markdown
Contributor

Per #86, this PR changes both the v0 and v1 init.py and rest_sessions.py so that extended information is placed in the User-Agent header. System information similar to pip is placed in every request and optional partner BE GEO ID and calling application/automation/whatever is set if present. An example is provided to pull data from https://api.meraki.com/api/vX/organizations/{organizationId}/apiRequests that outputs this extended data along with everything else into an easy to read CSV file.

@petecj2

petecj2 commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

Ugh, I missed one space in the v1 implementation. Specifically in rest_sessions.py line 106 there should be a space added to the static version string so that it's easier to separate it from the URL encoded JSON string. If you could fix as you accept the PR, I'd appreciate it. Sorry for the extra step.

Comment thread meraki_v0/rest_session.py Outdated
"name": platform.python_implementation(),
}

if user_agent_extended["implementation"]["name"] == 'CPython':

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole block can be shortened

        if user_agent_extended["implementation"]["name"] in ('CPython','Jython','IronPython'):
            user_agent_extended["implementation"]["version"] = platform.python_version()
        elif user_agent_extended["implementation"]["name"] == 'PyPy':
            if sys.pypy_version_info.releaselevel == 'final':
                pypy_version_info = sys.pypy_version_info[:3]
            else:
                pypy_version_info = sys.pypy_version_info
            user_agent_extended["implementation"]["version"] = ".".join(
                [str(x) for x in pypy_version_info]
            )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! The original was literally copied/pasted from the pip client User-Agent processing in an attempt to maintain some consistency with that. If we think that having more streamlined code is more important than that consistency, I'm fine changing it to the suggested block, but I wanted to provide some background as to why it is the way it is.

Thoughts? Consistency or streamlined?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't speak for pip, but my guess would be, that it was just a simple copy & paste or that the "platform.python_version()" methods were different in the past for the implementations.

Also would you mind to extract this user-agent part as a function? That way we could use it for the async implementation too without copy & paste =)

@petecj2

petecj2 commented May 4, 2020

Copy link
Copy Markdown
Contributor Author

Done, let me know if you'd like me to do anything else here.

@coreGreenberet

Copy link
Copy Markdown
Contributor

Thanks =D
I've just seen it now. You've written the version directly here. I would use the version variable instead, but everything else looks fine.

@petecj2

petecj2 commented May 4, 2020

Copy link
Copy Markdown
Contributor Author

The hard coded version string was there before my edits, so I didn't want to assume I should change it. If you'd like to do that though, go for it.

@shiyuechengineer
shiyuechengineer merged commit 9dd2941 into meraki:master May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants