/on/github/mdo/ has two records in elsewhere

[chadwhitacre]

db.one now raises an exception when there is more than one result for a query. Here is the first bug it's caught. How did mdo end up with two records in elsewhere? That's a bug.

https://www.gittip.com/on/github/mdo/

Stacktrace (most recent call last):

  File "aspen/website.py", line 76, in handle_safely
    response = self.handle(request)
  File "aspen/website.py", line 109, in handle
    response = request.resource.respond(request)
  File "aspen/resources/dynamic_resource.py", line 47, in respond
    exec self.pages[1] in context
  File "/app/www/on/github/%login/index.html.spt", line 18, in <module>
    user_info = github.get_user_info(path['login'])
  File "gittip/elsewhere/github.py", line 99, in get_user_info
    , (login,)
  File "postgres/__init__.py", line 463, in one
    return cursor.one(sql, parameters, default)
  File "postgres/cursors.py", line 105, in one
    out = self._some(sql, parameters, lo=0, hi=1)
  File "postgres/cursors.py", line 132, in _some
    raise TooMany(self.rowcount, lo, hi)
TooMany: Got 2 rows; expecting 0 or 1.

https://app.getsentry.com/gittip/gittip/group/7549412/

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

[zbynekwinkler]

We query by user_info['login'] but this value is never checked for uniqueness:

    rec = gittip.db.one( "SELECT user_info FROM elsewhere "
                     "WHERE platform='github' "
                     "AND user_info->'login' = %s"
                   , (login,)
                    )

Can you take a look at what exactly is in the 2 rows? We have these constraints in the place:

CONSTRAINT elsewhere_platform_participant_key UNIQUE (platform , participant ),
CONSTRAINT elsewhere_platform_user_id_key UNIQUE (platform , user_id )

I guess we have to different user_ids with the same user_info['login']. GitHub (and twitter as well) allow changing the login. So, if we check for uniqueness only the numeric user_id and not the login that would explain it. If we want to support urls of type /on/github/%login we should check the login for uniqueness as well as the user_id.

[zbynekwinkler]

BWT: how do I get access to the sentry page? I do not know what that is and I am not ready to let it write to my github or bitbucket account. Signing in with google and twitter is not working :(. [internal server error]

[chadwhitacre]

@zwn I just upgraded our Sentry account as part of #1506, and we're now on a plan that allows more than one user. If you can't get in now maybe I need to reinvite you?

[zbynekwinkler]

There are two rows with user_info->'login' = 'mdo'. Both have different uid. So my guess would be that some 'login' changes on the github side have caused this. To decide what should be done with this case we should put together some story for #1428 first.

[zbynekwinkler]

The same thing with twitter:
https://www.gittip.com/on/twitter/YesCT/
https://app.getsentry.com/gittip/gittip/group/8318202/

Stacktrace (most recent call last):

  File "aspen/website.py", line 76, in handle_safely
    response = self.handle(request)
  File "aspen/website.py", line 109, in handle
    response = request.resource.respond(request)
  File "aspen/resources/dynamic_resource.py", line 47, in respond
    exec self.pages[1] in context
  File "/app/www/on/twitter/%screen_name/index.html.spt", line 18, in <module>
    user_info = twitter.get_user_info(path['screen_name'])
  File "gittip/elsewhere/twitter.py", line 43, in get_user_info
    , (screen_name,)
  File "postgres/__init__.py", line 463, in one
    return cursor.one(sql, parameters, default)
  File "postgres/cursors.py", line 105, in one
    out = self._some(sql, parameters, lo=0, hi=1)
  File "postgres/cursors.py", line 132, in _some
    raise TooMany(self.rowcount, lo, hi)

[zbynekwinkler]

The reason for this is that we are not handling renames of unclaimed accounts. Claimed users are ok, because when they sign in we get theirs id, at which point we also update the user_info which handles the rename. There is no equivalent for this for unclaimed accounts (one of the mdos is unclaimed).

Google thinks that there is no way to resolve github id to login and certainly the way to do it is not obvious. Github provides http://developer.github.com/v3/users/#get-all-users to list all users that has a nifty paging parameter since that takes (you guessed it 🎉) id and lists all users with id > since. Giving it id-1 resolves id to login.

So when we get TooMany we should try to resolve all of them from id to login to take care of any renames (most likely within gittip/elsewhere/github.py#L89 and others).

Any takers?

[chadwhitacre]

Not sure why this is DevX. I'm removing that label.

[chadwhitacre]

Also, this is two stars, so let's wait until it's promoted to three stars before starting work on it, eh? :-)

[seanlinsley]

Uh, I thought the idea was to ideally only ever have a couple 3-star tickets, and to eventually run out of them?

[chadwhitacre]

@seanlinsley That was four-star tickets, when we were trying to think of stars as having bright-line definitions such as "Most users are getting errors" for four stars. The current priority system is based on gut and priority parties, not bright-line tests. If the site is burning, then the issue is off the star charts. :-)

[seanlinsley]

I guess my point is more that I don't imagine us running out of 3-star tickets, and unless our priorities change, I don't see individual tickets ever migrating. It seems much more likely that we'd, say, dedicate 50% of our individual time on 3-star tickets, 20% on 2-star, and 10% on 1-star. I'm not saying that we'd require that of people, that's just what think will end up happening.

[chadwhitacre]

unless our priorities change

I expect to periodically go through all tickets and reshuffle priorities like we did at the outset during the priority parties during the retreat. I'd hope the spread is more like 80 / 15 / 5, but yeah, not enforced.

[zbynekwinkler]

The api needed to resolve user_id for twitter accounts is https://dev.twitter.com/docs/api/1.1/get/users/lookup

[seanlinsley]

There's a newer Sentry grouping, https://app.getsentry.com/gittip/gittip/group/13527263/, that for some reason wasn't added to the existing one.

[zbynekwinkler]

This will be fixed by implementing #900.

[chadwhitacre]

This got fixed in #1369.

[Changaco]

No, it hasn't been fixed.

[chadwhitacre]

Indeed: https://app.getsentry.com/gratipay/gratipay-com/group/30121697/.

[chadwhitacre]

Closing in light of our decision to shut down Gratipay.

Thank you all for a great run, and I'm sorry it didn't work out! 😞 💃