awscli import fails: package_creator.display_name results in HTTP error 410: Gone

For every publishing record in Launchpad, we'd like to get the string of the name of the publication as viewed by Launchpad.

For git-ubuntu, using Launchpad this way may not be reproducible, as I think user accounts can have their display names changed at any time. It might be better for us to fall back to something deterministic such as being based on the source package. The ultimate fallback might have to be "Unknown <email address hidden>" or something.

/*$ !bash
{}
  {*}

Related: pkgbinarymangler has parsing difficulty on 70~lucid1

pkgbinarymangler 70~lucid1's changelog entry looks like this:

pkgbinarymangler (70~lucid1) lucid-backports; urgency=low

  * Automated backport upload; no source changes.

 -- pitti Fri, 09 Jul 2010 12:52:42 +0100

From Nish:
<nacc> it needs a try/except for that return code in importer.py:1646

FYI - There seems to be another affected package: python-tablib

Failed to import python-tablib (attempt 1/3)
git ubuntu import -l usd-importer-bot python-tablib
ERROR:root:Command exited 1: git ubuntu import -l usd-importer-bot python-tablib
ERROR:root:stdout:
ERROR:root:stderr: 08/05/2019 09:30:20 - INFO:Ubuntu Server Team importer v0.7.4
  08/05/2019 09:30:20 - INFO:Using git repository at /tmp/tmp7b_n6wix
  08/05/2019 09:30:28 - INFO:Importing patches-unapplied 0.12.1-1 to debian/sid
  Traceback (most recent call last):
    File "/snap/git-ubuntu/456/lib/python3.6/site-packages/gitubuntu/importer.py", line 1976, in import_publishes
      parent_overrides=parent_overrides,
    File "/snap/git-ubuntu/456/lib/python3.6/site-packages/gitubuntu/importer.py", line 1646, in import_unapplied_spi
      if spi.spphr.package_creator.display_name:
    File "/snap/git-ubuntu/456/lib/python3.6/site-packages/lazr/restfulclient/resource.py", line 732, in __getattr__
      return super(Entry, self).__getattr__(name)
    File "/snap/git-ubuntu/456/lib/python3.6/site-packages/lazr/restfulclient/resource.py", line 353, in __getattr__
      return self.lp_get_parameter(attr)
    File "/snap/git-ubuntu/456/lib/python3.6/site-packages/lazr/restfulclient/resource.py", line 237, in lp_get...

Another affected package is (python-)twisted

10/24/2019 08:38:47 - INFO:Importing patches-unapplied 16.4.1-2 to debian/sid
Traceback (most recent call last):
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/gitubuntu/importer.py", line 1976, in import_publishes
    parent_overrides=parent_overrides,
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/gitubuntu/importer.py", line 1646, in import_unapplied_spi
    if spi.spphr.package_creator.display_name:
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/lazr/restfulclient/resource.py", line 732, in __getattr__
    return super(Entry, self).__getattr__(name)
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/lazr/restfulclient/resource.py", line 353, in __getattr__
    return self.lp_get_parameter(attr)
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/lazr/restfulclient/resource.py", line 237, in lp_get_parameter
    self._ensure_representation()
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/lazr/restfulclient/resource.py", line 379, in _ensure_representation
    representation = self._root._browser.get(self._wadl_resource)
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/lazr/restfulclient/_browser.py", line 448, in get
    response, content = self._request(url, extra_headers=headers)
  File "/snap/git-ubuntu/458/lib/python3.6/site-packages/lazr/restfulclient/_browser.py", line 438, in _request
    raise error
lazr.restfulclient.errors.ClientError: HTTP Error 410: Gone
Response headers:
---
content-length: 42
content-type: text/plain
date: Thu, 24 Oct 2019 06:38:48 GMT
server: zope.server.http (HTTP)
status: 410
x-powered-by: Zope (www.zope.org), Python (www.python.org)
---
Response body:
---
b"u'User is suspended: ~python-modules-team'"
---

Currently seeing this for python-qt4. Betting a number of python modules are similarly affected.

Launchpad's display_name's are notoriously unreliable, I've run into this problem in other projects many times. Nish is right that this needs better handling around line 1646:

    fallback_author = None
    if spi.spphr.package_creator.display_name:
        fallback_author = str(spi.spphr.package_creator.display_name)

So, what we probably need is something akin to:

    fallback_author = None
    try:
        if spi.spphr.package_creator.display_name:
            fallback_author = str(spi.spphr.package_creator.display_name)
    except:
        try:
            fallback_author = spi.spphr.package_creator.name
        except:
            fallback_author = "Unknown"

1) It'd be better if it caught the specific exception being thrown, but I'm not sure what that is. KeyError is thrown when the user doesn't exist, but not sure what 'User suspended' is.

2) This falls back to the 'name' field, which is almost as problematic as the display_name. Also, since it's the user's launchpad username it may not be at all relevant from a package standpoint. But I don't know the intent of this code well enough to see if there's a better fallback.

3) Worst case it's setting it to "Unknown", but would something different (or just None) be better?

4) There is similar code in import_applied_spi that'll need the same fix. (And might be worth putting into a helper routine.)

Fwiw, technically this isn't a Launchpad service error; teams can be suspended for various reasons I suppose. So I don't think this needs filed against Launchpad

There might be something that could be done in LaunchpadLib though, to handle the 401 error but I think all that could be done would be to clarify the raised exception. I think the underlying issue has to be dealt with in importer.py.

A deeper problem here is that it affects hash stability. My intention has been to look at this after the importer-add-tests branch. The commit metadata needs specifying and should not depend on Launchpad state that might change. We then need tests for that and then will be able to fix the implementation to match.

Commit ab62e7d seems key to the history of "fallback" here. Based on the code before that commit, AFAICT the fallback was defensive programming rather than there being any specific edge case that we need to avoid. I think that means it's safe to specify something and replace how we determine the commit metadata with something deterministic.

Ah, no, actually it came from b0701e5 as a fix for bug 1638614. That's the edge case we need to ensure works.

Ah ok I see where this comes from now.

So yeah, both display_name and name are alterable, so for purposes of hash calculations they're not going to be reliable for hash stability.

If this code is used exclusively just for the hash, then would it be possible to just leave the name blank? Or 'Unknown' or some other deterministic string? If it must be something that can be connected to the author, could it just be a duplicate of the email address?

Another idea pops into mind of building a lookup table of author email:name, and then when a commit with a blank name is found, do a lookup to see if there is a matching alternate. However, while this would be more reliable than using launchpad, I think it'd only be marginally more deterministic; there'd still be corner cases, they'd just be smaller.

Oh, btw wondered if the fix for this might also result in a fix for #1728685?

> Oh, btw wondered if the fix for this might also result in a fix for #1728685?

It partially is, yes, but I'm going to focus on the author/commit metadata for now. I'll need to do the commit message separately but it won't be in this branch as the commit message comes in as a parameter.

Now published in edge and the importer is active with the fix. All imports which were affected are now caught up.