Permit pipenv and other tools to report itself as the downloader in pip's user agent

[theacodes]

This will allow pipenv to report itself as the downloader, which in turn, will allow us to measure pipenv adoption. This is marked as "trivial" because it's an internal-only change and the only project we want to use this is pipenv.

Context: pypa/packaging-problems#58 (comment)

[ncoghlan]

I was initially wondering if it may be better to structure this as two environment variables: PIP_USER_AGENT_INSTALLER_NAME and PIP_USER_AGENT_INSTALLER_VERSION

That way other pip-scripting clients like pip-compile and pip-sync could also set it, without everyone needing to be aware of the name/version string formatting that PyPI expects.

However, I then realised that there'd be a major downside of implementing this as a full override: it would mean we lost the information about which version of pip is being scripted, and hence about how well supported new sdist formats (etc) are actually going to be.

What if we instead added the wrapper name in square brackets, so that what you got on the PyPI side looked like:

pip[pipenv]/10.0.1

That should work without any Warehouse updates, or changes to folks Big Query queries, while still being able to provide some more granular info on which pip wrapper tools are seeing broad adoption.

[ncoghlan]

I decided to check my assumption about that square-bracket based format being compatible with linehaul already, and that turns out to be incorrect: https://github.com/pypa/linehaul/blob/master/linehaul/user_agents.py

The linehaul UA parser has a defined set of formats that it understands, and neither pipenv/ nor pip[pipenv]/ would match any of them.

Given that, I think we'd have to tackle this by:

1. Defining a "pip wrapper" format that appends extra info to the regular pip UA string (e.g. "pip/10.0.1 via pipenv/2018.5.18")
2. Add support for that format to linehaul
3. Add environment variable based support for that format to pip
4. Actually start setting those environment variables in wrapper tools

[theacodes]

The json that pip appends to the UA string also contains the pip version. We could also stick it in front, e.g. "pipenv/1.0.0 pip/10.0.0".

…

On Sat, May 19, 2018, 10:12 PM Nick Coghlan ***@***.***> wrote: I decided to check my assumption about that square-bracket based format being compatible with linehaul already, and that turns out to be incorrect: https://github.com/pypa/linehaul/blob/master/linehaul/user_agents.py The linehaul UA parser has a defined set of formats that it understands, and neither pipenv/ nor pip[pipenv]/ would match any of them. Given that, I think we'd have to tackle this by: 1. Defining a "pip wrapper" format that appends extra info to the regular pip UA string (e.g. "pip/10.0.1 via pipenv/2018.5.18") 2. Add support for that format to linehaul 3. Add environment variable based support for that format to pip 4. Actually start setting those environment variables in wrapper tools — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5424 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUc7FbG_-hsUw8biidAviWvAeGkORFks5t0PtGgaJpZM4UEBhM> .

[pradyunsg]

"pip/10.0.1 via pipenv/2018.5.18"

I like this format. On the BigQuery side of things, if we can, it should probably result in mirroring details.installer's schema, adding something for the wrapper tool -- details.wrapper maybe?

@dstufft is likely the best guy for commenting on this. :)

[ncoghlan]

I'd actually forgotten that the UA in modern pip versions included a JSON blob after the initial "pip/version" UA marker.

Given that, we shouldn't actually need to modify the UA prefix at all - instead, we'd submit a PR to update linehaul's UserAgent schema to include an extra optional wrapper = pyrsistent.field(type=Installer, factory=Installer.create) field, then after that had been deployed, update the JSON data in pip's UA to include the extra subfield with the wrapper info based on the PIP_USER_AGENT_WRAPPER_NAME and PIP_USER_AGENT_WRAPPER_VERSION environment variables.

@dstufft Does ^^^ sound like a workable approach to you?

[conspicuousClockwork]

There is also utility for those who need to modify the user-agent for edge cases. For example, all GET requests on a proxy I am currently using requires the header to contain a specific string anywhere in it.

Would it be plausible to add an extra subfield to the JSON, so the user may add in their own, even if their wrapper of preference is already utilizing PIP_USER_AGENT_WRAPPER_NAME and PIP_USER_AGENT_WRAPPER_VERSION?

PIP_USER_AGENT_PROVISIONAL_STRING Maybe?

@ncoghlan if this sounds okay to you I'd like to open a seperate issue and PR since this one seems to require changes across both linehaul and pip and is more for the use of wrappers instead of users.

[BrownTruck]

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

[theacodes]

Hey all, is this still useful? I'm happy to rebase and such but I want to make sure it doesn't die on the vine again.

[pradyunsg]

I think so. @dstufft is likely the best person for this. Maybe pinging him over email would help.

[theacodes]

Okay, this is rebased. @dstufft please take a look when you can?

[cjerdonek]

Being able to add context like this does seem useful, but I'd like to hear thoughts on @ncoghlan's idea above to include the info in the JSON instead (what he is calling the "wrapper" info).

One concern I'd have with letting people set the leading characters to an arbitrary string is that it would break the guarantee that pip's user agent string will be parseable. Also, even if the info about pip is in the JSON, linehaul only knows to parse the JSON if the string starts with pip/ in the first place:
https://github.com/pypa/linehaul/blob/420354cf789b064f0d38ce02573f6af51aa0306a/linehaul/ua/parser.py#L43-L54

It looks like linehaul has to add custom code for each leading prefix it recognizes (with special logic for each), so it seems like letting the leading prefix vary would break that model:
https://github.com/pypa/linehaul/blob/420354cf789b064f0d38ce02573f6af51aa0306a/linehaul/ua/parser.py#L92

[theacodes]

I'm totally fine either way.

…

On Tue, Feb 26, 2019, 3:54 AM Chris Jerdonek ***@***.***> wrote: Being able to add context like this does seem useful, but I'd like to hear thoughts on @ncoghlan <https://github.com/ncoghlan>'s idea above <#5424 (comment)> to include the info in the JSON instead (what he is calling the "wrapper" info). One concern I'd have with letting people set the leading characters to an arbitrary string is that it would break the guarantee that pip's user agent string will be parseable. Also, even if the info about pip is in the JSON, linehaul only knows to parse the JSON if the string starts with pip/ in the first place: https://github.com/pypa/linehaul/blob/420354cf789b064f0d38ce02573f6af51aa0306a/linehaul/ua/parser.py#L43-L54 It looks like linehaul has to add custom code for each leading prefix it recognizes (with special logic for each), so it seems like letting the leading prefix vary wouldn break that model: https://github.com/pypa/linehaul/blob/420354cf789b064f0d38ce02573f6af51aa0306a/linehaul/ua/parser.py#L92 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5424 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAPUc-hBoodbCNZ46k6Rge9wLMehIba1ks5vRSBugaJpZM4UEBhM> .

[BrownTruck]

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

[pradyunsg]

@cjerdonek let's close this, given #5549?

[cjerdonek]

@pradyunsg This is a different request / different purpose. The string in PR #5549 is meant to be ignored by linehaul / statistics reports, whereas this PR is meant to make pipenv visible to the stats...

[cjerdonek]

In my mind, the next step on this PR is to follow what @ncoghlan suggested above, namely--

Given that, we shouldn't actually need to modify the UA prefix at all - instead, we'd submit a PR to update linehaul's UserAgent schema to include an extra optional wrapper = pyrsistent.field(type=Installer, factory=Installer.create) field,

The reason is that for the additional "wrapper" info to be useful, we need to have buy-in from linehaul that it will actually parse and report this info.

[cjerdonek]

FYI, I updated this issue's title to reflect the underlying goal of the issue rather than one possible implementation. This also helps to reduce the superficial similarity with PR #5549.

[chrahunt]

I created #7186 for implementation of the solution proposed by @ncoghlan above, and pypi/linehaul#50 for the first step. I will close this PR and we can do any necessary followup there.