gerrit.googlesource.com shows little face photos next to usernames, which is a good way for making it quicker and less mental effort to recognize / spot people, and also for humanizing the interface. We should do that too.
The simplest solution seems to be to install the avatars-gravatar plugin.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Declined | None | T191183 Enable avatars in gerrit | |||
| Invalid | None | T193216 Document how users set avatars in on-wiki gerrit documentation |
Change 439939 had a related patch set uploaded (by Paladox; owner: Paladox):
[operations/puppet@production] Add gerrit.wmfusercontent.org to common/cache/misc.yaml
Change 439939 merged by Dzahn:
[operations/puppet@production] cache::misc: Add gerrit backend, gerrit.wmfusercontent.org
gerrit.wmfusercontent.org now exists in cache::misc and requests would be forwarded to cobalt as the backend.
This unblocked this to a certain extent because avatars could now be hosted on gerrit.wmfusercontent.org (akin to phab.wmfusercontent.org is used as a separate domain for user uploads, for security reasons).
Next would be an Apache virtual host on cobalt to handle it.
gerrit.wikimedia.org itself is _not_ behind cache::misc but serves traffic directly with a public IP.
Change 439783 had a related patch set uploaded (by Paladox; owner: Paladox):
[operations/puppet@production] Gerrit: Add support for avatars url in apache
Change 439808 had a related patch set uploaded (by Paladox; owner: Paladox):
[operations/puppet@production] Gerrit: Hook up gerrit.wmfusercontent.org to apache
Change 440104 had a related patch set uploaded (by Paladox; owner: Paladox):
[operations/puppet@production] Gerrit: Clone avatars repo into /var/www/avatars
Change 424708 abandoned by Paladox:
Gerrit: Add url for avatars and setups gerrit.wmfusercontent.org
@thcipriani or @mmodell wondering if you be able to comment here that releng supports this avatar change and maintaining it please. (Ops need releng to comment)
Change 440104 merged by Dzahn:
[operations/puppet@production] Gerrit: Clone avatars repo into /var/www/gerrit-avatars
Change 456437 had a related patch set uploaded (by Paladox; owner: Paladox):
[operations/puppet@production] Gerrit: Setup avatars url in gerrit config
Change 439783 merged by Dzahn:
[operations/puppet@production] Gerrit: Add support for avatars url in apache
Change 456514 had a related patch set uploaded (by Paladox; owner: Paladox):
[operations/puppet@production] gerrit: Index avatars url showing directory listings
Change 456514 merged by Dzahn:
[operations/puppet@production] gerrit: redirect avatars_host root URL to regular gerrit
Change 439808 merged by Dzahn:
[operations/puppet@production] Gerrit: Hook up gerrit.wmfusercontent.org to apache
Mentioned in SAL (#wikimedia-operations) [2018-10-05T20:44:16Z] <mutante> gerrit - adding gerrit.wmfusercontent.org virtual host for avatars. applied first on gerrit2001, then on cobalt (T191183)
Mentioned in SAL (#wikimedia-operations) [2018-10-05T20:48:52Z] <mutante> T191183 - it's still showing the error page as before but that isn't due to apache issues, it just needs additional ferm rules
Change 464907 had a related patch set uploaded (by Dzahn; owner: Paladox):
[operations/puppet@production] Gerrit: Change backend for gerrit in varnish
Change 464907 merged by BBlack:
[operations/puppet@production] Gerrit: Change backend for gerrit in varnish
The prior conversation at T179212 may be relevant here. The two options discussed so far were 1) Use Gerrit repo with static file server, or 2) Use Phabricator.
Benefits:
Drawback:
(via a cacheable CGI script, or from a static directory populated periodically by a cron job script).
Benefits:
Drawbacks:
I don't think that cloning / committing / pushing is a big hurdle in this case. After all we are talking about Gerrit users not Phabricator. People who already explicitly come to do code review will be familiar with using git.
note that the avatar would have to match the users username in gerrit. So using phabricator would not work seeing as the name of the file does not match the users username.
The 100x100 size isn't what currently happens in the repository:
$ identify *.png default.png PNG 425x425 425x425+0+0 8-bit Grayscale sGray 13916B 0.000u 0:00.007 ladsgroup.png PNG 640x640 640x640+0+0 8-bit sRGB 228c 123552B 0.000u 0:00.000 niedzielski.png PNG 350x350 350x350+0+0 8-bit sRGB 150395B 0.000u 0:00.000 paladox.png PNG 400x400 400x400+0+0 8-bit sRGB 109c 2564B 0.000u 0:00.000
(the size are after optipng -o7 to give a fair representation of what we could expect)
Change 465149 had a related patch set uploaded (by Dereckson; owner: Dereckson):
[All-Avatars@master] Optimize the PNG assets
Can you explain what you mean here?
The Phabricator export script does not exist yet. There are no files yet to have names. If and when we write this script, of course, we use the correct file name.
@Krinkle would you know how to build this? Seeing as you can link your mediawiki profile or your ldap account, you could have a different name.
Yes, you can. You can also link it to a Developer account (LDAP), which is what Gerrit uses. So the proxy script or export script, would query Phabricator for a user that has the Gerrit name as LDAP link. And if there is one match, use it.
We already talked about this a few months ago on IRC, and at T179212
Yes, I think anyone involved around this task could do it. It's not a question of how.
The question is, what do we want for the user experience, and would it be worth it to write a little script for it or not.
Really? As far as I know all gravatar integrations send hashes of emails. This means they don't know which email it is, unless it exists in their system (assuming they don't crawl email addresses from elsewhere). Having said that, it still doesn't sound a very good idea.
There are a few troubles:
gerrit.avatars.changeUrl and an end point on Phabricator side seems to the easiest solution as well as being user friendly (one place to update it). ver Ideally we would add the information to LDAP to have the avatar at a central place and reused everywhere (wiki, Phabricator, Gerrit), but that is over the current scope.
Gravatar hashes are plain unsalted md5 so not really useful for protecting your email address - it can just be plugged into one of the reverse email hashing services to get the raw address back.
This is probably something we should enforce somehow (jenkins? some tool to be created to upload?) before exposing this feature broadly: optimization of uploaded images and enforcement of the size of these images to help mitigate the problem of this repo growing "uncontrollably large".
Also, should we be using git lfs (or maybe just not git at all) for this repo? The files themselves are pretty tiny, but I don't think there's any advantage to storing old avatars as binary data in a repo folks are expected to clone and push. For instance, the optimization patch above, while necessary for serving these images on the web actually made the repo itself larger since now we have the old default.png (28K .git/objects/ec/e17874be5f0ae77c25c0dd672a3c71eebb536c) and the new default.png (14K .git/objects/94/d4ec77c43cab9d3d9952ca00c46a4e8878f711). Having lots of users use this repo and having lots of users updating their avatars over time seems like it would make this repo hard to work with and generally fulfill the "uncontrollably large" prophecy.
Yes.
Also, should we be using git lfs (or maybe just not git at all) for this repo?
Yes.
We won't be able to use git lfs on cobalt as it is using jessie whereas the git-lfs package is in stretch-backports+.
We could enforce it so users can only upload there image through a tool hosted on tool forge. So that way it can run the image optimising tool and upload.
We could modify this https://github.com/valhallasw/gerrit-patch-uploader/ or fork it.
That's simply a technical issue to work through, not a complete blocker.
We could enforce it so users can only upload there image through a tool hosted on tool forge. So that way it can run the image optimising tool and upload.
We could modify this https://github.com/valhallasw/gerrit-patch-uploader/ or fork it.
This goes back to Krinkle's question of what we want the user experience to be.
UX-wise a single central place for profile images is obviously preferable, so using Phabricator makes a lot of sense. (Having some way to store a profile image in your Wikimedia central account would make even more... maybe in the next decade :) OTOH that would mean you can't use a different image, which could be problematic as the avatar size is very different between Phabricator (several sizes, but most commonly 50x50 in comment threads and 140x140 on hover) and Gerrit (16x16) so the same image might not work well.
Also, Conduit is not confidence-inspiring though, IMO - poorly documented (if at all) and in constant churn. Most importantly, it's not very fast, so the Gerrit plugin would still have to store images somewhere. At that point, maybe just cut out Phabricator and manage that store directly, and make it easy to transfer the Phabricator profile image? (I agree that git is not the most convenient or frugal backend for that.)
If gerrit only shows 16x16 images, I would argue that it's not even worth doing. At that size, avatars will be difficult to distinguish and that will minimize any benefits gained by having them.
Gerrit does not only do 16x16 (it does larger images) and in polygerrit's ui, it can make images look bigger or smaller. It all depends on where you are in the interface, the image is larger on the settings page, but only a bit smaller on the change screen. See https://gerrit-review.googlesource.com/c/gerrit/+/209453
Yeah, anything that queries Phabricator or LDAP in our infra probably shouldn't go without caching. Either Varnish or local Nginx could suffice for that.
That would still exposes a subset of users to the slow request when populating the cache. For that UX performance reason, as well as to eliminate all factors of externally induced load more generally, I recommended earlier to instead have an offline script (e.g. cron) do this periodically. That way, the only thing we expose is a stateless and static file server, with no persistent or versioned storage anywhere. The scripts lets Phabricator generate the thumbs and merely writes them to a working directory. It could do this once an hour or on a loop, controlled by cron via puppet, so either from both puppet or ssh, an admin can disable the script at any time for any reason.
Well the effort was inflated by dubious privacy requirements :) I still think the best course of action would be gravatar with a proxy.
Change 545066 had a related patch set uploaded (by Paladox; owner: Paladox):
[operations/puppet@production] Gerrit: Remove All-Avatars from gerrit's module
Change 545066 merged by Dzahn:
[operations/puppet@production] Gerrit: Remove All-Avatars from gerrit's module
^ The reason to merge this was not a comment on the general question to enable avatars. The reason was that during T222391 we noticed an undesirable dependency. During a Gerrit migration to a new server the existing code meant puppet would try to git pull from avatars repo, fail to do so and then not create the needed Apache site config due to a dependency. So "if avatars repo is not reachable then we don't get Apache config for gerrit.wikimedia.org itself". We had to manually copy config files from the old host to fix that.
Adding the few project tags we are using nowadays.
Lowering priority since clearly we have no bandwith to work on adding avatar support.
I note this is a self-inflicted problem: the gravatar plugin would work out of the box, and the privacy issues raised before are over-exaggerated, to put it mildly (since the email addresses are already public).
I've come around to agreeing with @Tgr. Gravatar seems like something we could support in good conscience.
The company operating it (Automattic) is one we've dealt with before and wouldn't mind referring to in documentation around setting up your avatar.
Traffic-wise we'd want to proxy it of course, but that seems quite do-able. We could also set a low backend timeouts in Varnish (connect_timeout, first_byte_timeout, between_bytes_timeout) to enforce a strict SLA. Then fail-fast with 404, 503, or even with a static placeholder response (e.g. inline synthetic response with 1px transparent gif).
Change 615514 had a related patch set uploaded (by Dzahn; owner: Dzahn):
[operations/puppet@production] ATS: remove gerrit.wmfusercontent.org
Change 615557 had a related patch set uploaded (by Dzahn; owner: Dzahn):
[operations/dns@master] remove gerrit.wmfusercontent.org
Change 615514 merged by Dzahn:
[operations/puppet@production] ATS: remove gerrit.wmfusercontent.org
Change 615557 merged by Dzahn:
[operations/dns@master] remove gerrit.wmfusercontent.org
The summary is:
We can't use the third party service gravatar.com since that leaks personal information to a third party.
An idea was to reuse the profile image from Phabricator or a Gerrit repository to point to (see T191183#4647076 above), that is not funded and not not worked on.
As such as declining the task for now.
I still think that is FUD. All we'd need is to set up a local instance of gravatar-privacy-proxy, a fairly trivial PHP microservice.
(Not wanting to nit-pick, really, just curious...)
Is sending username/email to gravatar by default (how I assume this would work using that local proxy) considered personal information? We are telling gravatar that a specific user is using our service. ¯\_(ツ)_/¯
Sorta responding to Greg and hashar too. I agree having proxy is less than optimal. But there's another option as suggested in T256541: Fix the problem with gravatar and mailman3 (gravatars are useful for the new mailman too). You can have a gravatar service too T256541#6371029 I sorta like having a central profile place for wikimedia.
In general yes, but Gerrit makes that information public anyway; something like https://gerrit.wikimedia.org/r/accounts/?o=DETAILS&q=is:active+OR+is:inactive will happily list all users with their emails.
Just for the context the reason I declined this again is because I have seen on IRC a notice about disabling gravatar for OTRS ( T187984#6465324 ) which remembered me about this task.
Yes indeed, that would most probably addresses the private information leak. Then even if the service is trivial, bringing it to production is a significant amount of work that adds up to our pile.
Now, we have OTRS 6, Gerrit and Mailman v3 having a use case for Gravatar. Which seems worth filing a new project for it and find resources to set it up and offer a shared service. By declining the task I am merely stating it out of scope for Gerrit itself.
Filed T263161: Set up a Gravatar proxy in Wikimedia production about having a Gravatar proxy.
Change 456437 abandoned by Hashar:
[operations/puppet@production] Gerrit: Setup avatars url in gerrit config
Reason:
I have declined the attached task T191183, there is no funding to implement the avatar feature.
Coming back to this again... since the Gravatar issue (T263161) is unlikely to move forward any time soon, wondering if we could pick up the avatars-external plugin solution and try it out?
Essentially no. That plugins lets one get Gerrit to point to a URL that would have a the username inserted (which is the login from Wikitech/LDAP) but we do not have any infrastructure to let people upload their avatar. Maybe if one day MediaWiki supports attaching an avatar to a user we could then have Gerrit to load it from something such as https://wikitech.wikimedia.org/wiki/Special:Avatar/%s where %s would be replaced by the username.
The patch proposes to use a repo (https://gerrit.wikimedia.org/g/All-Avatars/+/refs/heads/master), would that work, at least as an experiment?
Maybe if one day MediaWiki supports attaching an avatar to a user we could then have Gerrit to load it from something such as https://wikitech.wikimedia.org/wiki/Special:Avatar/%s where %s would be replaced by the username.
That would be nice, but doesn't sound like it would happen any time soon. (I think T128953 is the relevant task, and it hasn't been touched in years.)
Personally, I don't think that will ever happen. AFAIK the best summary of likely-problems is at https://www.mediawiki.org/wiki/User:Isarra/Avatars#So_many_ways_this_could_go_horribly_wrong (which was written about Flow, hence the preamble there).
Mentioned in SAL (#wikimedia-releng) [2024-01-15T15:25:23Z] <hashar> gerrit: archived All-Avatars.git # T191183