Receive package claim link-back #4397

chadwhitacre · 2017-03-31T16:12:38Z

Part of #4305, follow-on from #4335.

Requirements

(9) from the workflow … the big one!

(9) hit /on/npm/react/claim

Todo

finish out email tests now that package claiming API is in place
make sure we disallow already claimed packages from being claimed
test when {email,nonce} == ' '

chadwhitacre · 2017-03-31T16:15:03Z

I think this is a change to verify_email, I don't think we actually need to touch verify.html.spt.

chadwhitacre · 2017-03-31T16:24:16Z

verify_email is a bit of a dense logic puzzle. Kinda nervous about tinkering with it.

chadwhitacre · 2017-03-31T16:25:09Z

/me takes a deep breath ...

chadwhitacre · 2017-04-03T18:54:00Z

Started a todo. 😊

Todo

???

chadwhitacre · 2017-04-03T22:02:27Z

Yesssss! 05ff710 is the heart of claiming. Now to dig out!

chadwhitacre · 2017-04-14T12:36:30Z

Taking out of review until I can rebase now that #4335 is in ...

chadwhitacre · 2017-04-17T12:07:20Z

Squashed and rebased onto #4305 now that #4335 is in, was 757b498. Ready for review!

dowski

Looks good overall - just a few comments.

dowski · 2017-04-19T01:49:38Z

gratipay/models/package/__init__.py

+        return self.load_team(self.db)
+
+
+    def load_team(self, cursor):


Should this be _load_team, or do you intend for this method to be part of the public API for this class?

Changed to _load_team in 404107c.

I think you mean 79fa918.

dowski · 2017-04-19T01:52:27Z

gratipay/models/package/__init__.py

+        linked to this package if there is one, otherwise ``None``. Makes a
+        database call.
+        """
+        return self.load_team(self.db)


I guess self.db is a cursor. It's interesting to see in the class below that it isn't a cursor (or maybe it is, but you do call get_cursor on it).

self.db exposes the same run, one, all API as a cursor does, but with less control over the transaction. Docs.

dowski · 2017-04-19T01:52:51Z

gratipay/models/participant/email.py

+        if cursor:
+            self._set_primary_email(email, cursor)
+        else:
+            with self.db.get_cursor() as cursor:


This is the place where I'm talking about the get_cursor call.

Yeah, this is a bit of a goofy construction. liberapay/postgres.py#40 would help.

dowski · 2017-04-19T01:55:14Z

gratipay/models/participant/email.py

-        except IntegrityError:
-            return VERIFICATION_STYMIED
+        with self.db.get_cursor() as cursor:
+            record = self.get_email(email, cursor)


Hooray for replacing r with record!

dowski · 2017-04-19T02:00:42Z

gratipay/models/package/__init__.py

+        return cursor.one('SELECT t.*::teams FROM teams t WHERE t.id=%s', (self.team_id,))
+
+
+    def get_or_create_linked_team(self, cursor, owner):


You can just make this create_team or ensure_team and return None. You only use the return value in your tests where you can do a separate load_team to do your assertions.

Done in 404107c.

I think you mean 79fa918.

Picks up at #4404 (comment).

Erm, #4404 (comment) is where it becomes useful to have a team object returned (when we implement multi-package claiming). You okay to keep that here, then?

dowski · 2017-04-19T02:03:20Z

gratipay/models/package/__init__.py

+        team = self.load_team(cursor)
+        if not team:
+            slug = str(uuid.uuid4()).lower()
+            team = Team.insert( slug=slug


Is there an equivalent of INSERT OR IGNORE INTO teams ... that you can use here? Then you can skip the load_team call above.

TIL about INSERT OR IGNORE. Looks like that's a MySQL thing?

The alternative with Postgres is to try/except, but that doesn't work with cursors. I tried! :-)

The modern way in postgres ≥ 9.5 is INSERT ... ON CONFLICT DO NOTHING (https://www.postgresql.org/docs/9.5/static/sql-insert.html). It's possible to do the same thing with older versions of postgresql even inside a cursor by using pgSQL instead of plain SQL (here's an example of UPSERT with pgSQL).

INSERT OR IGNORE is SQLite (https://sqlite.org/lang_insert.html).

@dowski Do you think it's worth the gymnastics here, or are you okay with the extra db call? If concurrency ends up being an issue that could change the equation ...

dowski · 2017-04-19T14:00:49Z

gratipay/models/package/__init__.py

+    def ensure_team(self, cursor, owner):
+        """Given a db cursor and :py:class:`Participant`, insert into ``teams`` if need be.
+        """
+        team = self._load_team(cursor)


Is there an implicit transaction here? What if this method is called concurrently? Seems like you could wind up with multiple entries with different UUIDs.

" ... called concurrently with the same input."

If cursors is actually self.db then yes, there is an implicit transaction under here. However, the purpose of exposing the cursor parameter is precisely to allow passing in a proper cursor, so that an entire call chain can operate within one transaction to preserve integrity. In some methods cursor is optional (e.g.), but cursor is required and front-loaded here because integrity is an issue here.

The transaction of which ensure_teams is a part begins in finish_email_verification. If finish_email_verification is called concurrently with the same input, then ... I don't know. I've been working most of the day on trying to sort this out. What I have so far is in 8da4f9c.

Here's the output I'm seeing:

pid-10149 thread-4563636224 (Thread-2) 1492636220.45 loading team pid-10149 thread-4563636224 (Thread-2) 1492636220.46 old team: n/a pid-10149 thread-4563636224 (Thread-2) 1492636220.46 slug: 0d8f713b-fc8b-43f8-ad78-638a8d0808c3 pid-10149 thread-4563636224 (Thread-2) 1492636220.47 new team: 0d8f713b-fc8b-43f8-ad78-638a8d0808c3 pid-10149 thread-4563636224 (Thread-2) 1492636220.47 updated, sleeping ... pid-10149 thread-4563636224 (Thread-2) 1492636222.47 slept pid-10149 thread-4567842816 (Thread-3) 1492636222.48 loading team pid-10149 thread-4567842816 (Thread-3) 1492636222.49 old team: n/a pid-10149 thread-4567842816 (Thread-3) 1492636222.49 slug: ef264dc3-7d62-44d4-a907-ab18c86afa32 pid-10149 thread-4567842816 (Thread-3) 1492636222.49 new team: ef264dc3-7d62-44d4-a907-ab18c86afa32 pid-10149 thread-4567842816 (Thread-3) 1492636222.49 updated, sleeping ... pid-10149 thread-4567842816 (Thread-3) 1492636224.49 slept F ================================================== FAILURES ================================================== ______________________ PackageLinking.test_finishing_email_verification_is_thread_safe _______________________ Traceback (most recent call last): File "/Users/whit537/personal/gratipay/gratipay.com/tests/py/test_email.py", line 809, in test_finishing_email_verification_is_thread_safe assert results[b.ident] == _email.VERIFICATION_FAILED AssertionError: assert 5 == 1 + where 1 = _email.VERIFICATION_FAILED

That looks to me like there's something hidden from view that is synchronizing the threads. Even though I start them in parallel, they appear to run in sequence. Hmm ... 🤔

Ah! I think it's because the ... well, wait ...

I was gonna say because the calls synchronize on the cursor.run('DELETE ...') call (why that one instead of the SELECTs? Is psycopg2/Postgres being fancy?), so that the second cursor sees a certain state following the lock release, ... but wouldn't a second insert on teams_to_packages still fail?

Here we go: team is not None the second time around.

tests/py/test_email.py pid-36042 thread-4463992832 (Thread-2) have cursor, will travel pid-36042 thread-4463992832 (Thread-2) have record:Record(id=118, address=u'[email protected]', verified= None, nonce=u'fccf6dec-7559-4cb2-ba77-046f2aa0efe7', verification_start=datetime.datetime(2017, 4, 24, 13, 44, 6, 588594, tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None)), verification_end=None, participant_id=1L) pid-36042 thread-4463992832 (Thread-2) have packages:[<gratipay.models.package.Package object at 0x109d09250>] pid-36042 thread-4463992832 (Thread-2) let's finish up ... pid-36042 thread-4468199424 (Thread-3) have cursor, will travel pid-36042 thread-4463992832 (Thread-2) finishing up ... pid-36042 thread-4463992832 (Thread-2) team: None pid-36042 thread-4468199424 (Thread-3) have record:Record(id=118, address=u'[email protected]', verified=None, nonce=u'fccf6dec-7559-4cb2-ba77-046f2aa0efe7', verification_start=datetime.datetime(2017, 4, 24, 13, 44, 6, 588594, tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None)), verification_end=None, participant_id=1L) pid-36042 thread-4468199424 (Thread-3) have packages:[<gratipay.models.package.Package object at 0x109d12250>] pid-36042 thread-4463992832 (Thread-2) inserting into teams_to_packages: 45 95 pid-36042 thread-4468199424 (Thread-3) let's finish up ... pid-36042 thread-4463992832 (Thread-2) inserted into teams pid-36042 thread-4463992832 (Thread-2) [Record(team_id=45L, package_id=95L)] pid-36042 thread-4468199424 (Thread-3) finishing up ... pid-36042 thread-4468199424 (Thread-3) team: 80e34bf6-324a-4b63-a700-c3fe3b624001

diff --git a/gratipay/models/package/__init__.py b/gratipay/models/package/__init__.py index a0591a1..9a69037 100644 --- a/gratipay/models/package/__init__.py +++ b/gratipay/models/package/__init__.py @@ -65,7 +65,10 @@ class Package(Model): def ensure_team(self, cursor, owner): """Given a db cursor and :py:class:`Participant`, insert into ``teams`` if need be. """ + from aspen import log + log('finishing up ...') team = self._load_team(cursor) + log('team: {}'.format(team.name if team else 'None')) if not team: slug = str(uuid.uuid4()).lower() team = Team.insert( slug=slug @@ -76,6 +79,7 @@ class Package(Model): , owner=owner , _cursor=cursor ) + log('inserting into teams_to_packages: {} {}'.format(team.id, self.id)) cursor.run('INSERT INTO teams_to_packages (team_id, package_id) ' 'VALUES (%s, %s)', (team.id, self.id)) from aspen import log diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index e47cafe..5ae2528 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -228,13 +228,17 @@ class Email(object): def finish_email_verification(self, email, nonce): + from aspen import log if '' in (email.strip(), nonce.strip()): return VERIFICATION_MISSING with self.db.get_cursor() as cursor: + log('have cursor, will travel') record = self.get_email(email, cursor) + log('have record:' + str(record)) if record is None: return VERIFICATION_FAILED packages = self.get_packages_claiming(cursor, nonce) + log('have packages:' + str(packages)) if record.verified and not packages: assert record.nonce is None # and therefore, order of conditions matters return VERIFICATION_REDUNDANT @@ -243,6 +247,7 @@ class Email(object): if (utcnow() - record.verification_start) > EMAIL_HASH_TIMEOUT: return VERIFICATION_EXPIRED try: + log('let\'s finish up ...') self.finish_package_claims(cursor, nonce, *packages) self.save_email_address(cursor, email) cursor.connection.commit()

Gosh. Is there a different possible behavior for each cursor call? 😳

https://www.postgresql.org/docs/9.3/static/explicit-locking.html

Woo-hoo! 💃 ✊

tests/py/test_email.py pid-40671 thread-4572102656 (Thread-2) have cursor, will travel pid-40671 thread-4572102656 (Thread-2) have record:Record(id=121, address=u'[email protected]', verified=None, nonce=u'3193076d-96ce-4864-b927-d5dedb9ff75c', verification_start=datetime.datetime(2017, 4, 24, 14, 6, 58, 165862, tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None)), verification_end=None, participant_id=1L) pid-40671 thread-4572102656 (Thread-2) have packages:[<gratipay.models.package.Package object at 0x110421210>] pid-40671 thread-4572102656 (Thread-2) let's finish up ... pid-40671 thread-4576309248 (Thread-3) have cursor, will travel pid-40671 thread-4572102656 (Thread-2) finishing up ... pid-40671 thread-4572102656 (Thread-2) team: None pid-40671 thread-4572102656 (Thread-2) inserting into teams_to_packages: 47 98 pid-40671 thread-4572102656 (Thread-2) inserted into teams pid-40671 thread-4572102656 (Thread-2) [Record(team_id=47L, package_id=98L)] pid-40671 thread-4576309248 (Thread-3) have record:Record(id=121, address=u'[email protected]', verified=True, nonce=None, verification_start=datetime.datetime(2017, 4, 24, 14, 6, 58, 165862, tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None)), verification_end=datetime.datetime(2017, 4, 24, 14, 6, 58, 192552, tzinfo=psycopg2.tz.FixedOffsetTimezone(offset=0, name=None)), participant_id=1L) pid-40671 thread-4576309248 (Thread-3) have packages:[] . ================================ 86 tests deselected by '-kthread_safe' ================================= ================================ 1 passed, 86 deselected in 3.54 seconds ================================

diff --git a/gratipay/models/package/__init__.py b/gratipay/models/package/__init__.py index a0591a1..9a69037 100644 --- a/gratipay/models/package/__init__.py +++ b/gratipay/models/package/__init__.py @@ -65,7 +65,10 @@ class Package(Model): def ensure_team(self, cursor, owner): """Given a db cursor and :py:class:`Participant`, insert into ``teams`` if need be. """ + from aspen import log + log('finishing up ...') team = self._load_team(cursor) + log('team: {}'.format(team.name if team else 'None')) if not team: slug = str(uuid.uuid4()).lower() team = Team.insert( slug=slug @@ -76,6 +79,7 @@ class Package(Model): , owner=owner , _cursor=cursor ) + log('inserting into teams_to_packages: {} {}'.format(team.id, self.id)) cursor.run('INSERT INTO teams_to_packages (team_id, package_id) ' 'VALUES (%s, %s)', (team.id, self.id)) from aspen import log diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py index e47cafe..bd6260d 100644 --- a/gratipay/models/participant/email.py +++ b/gratipay/models/participant/email.py @@ -228,13 +228,17 @@ class Email(object): def finish_email_verification(self, email, nonce): + from aspen import log if '' in (email.strip(), nonce.strip()): return VERIFICATION_MISSING with self.db.get_cursor() as cursor: - record = self.get_email(email, cursor) + log('have cursor, will travel') + record = self.get_email(email, cursor, and_lock=True) + log('have record:' + str(record)) if record is None: return VERIFICATION_FAILED packages = self.get_packages_claiming(cursor, nonce) + log('have packages:' + str(packages)) if record.verified and not packages: assert record.nonce is None # and therefore, order of conditions matters return VERIFICATION_REDUNDANT @@ -243,6 +247,7 @@ class Email(object): if (utcnow() - record.verification_start) > EMAIL_HASH_TIMEOUT: return VERIFICATION_EXPIRED try: + log('let\'s finish up ...') self.finish_package_claims(cursor, nonce, *packages) self.save_email_address(cursor, email) cursor.connection.commit() @@ -295,15 +300,19 @@ class Email(object): ) - def get_email(self, address, cursor=None): + def get_email(self, address, cursor=None, and_lock=False): """Return a record for a single email address on file for this participant. + + :param unicode address: the email address for which to get a record + :param Cursor cursor: a database cursor; if ``None``, we'll use ``self.db`` + :param and_lock: if True, we will acquire a write-lock for you on the row + :returns: a database record (named tuple) + """ - return (cursor or self.db).one(""" - SELECT * - FROM emails - WHERE participant_id=%s - AND address=%s - """, (self.id, address)) + sql = 'SELECT * FROM emails WHERE participant_id=%s AND address=%s' + if and_lock: + sql += ' FOR UPDATE' + return (cursor or self.db).one(sql, (self.id, address)) def get_emails(self):

chadwhitacre · 2017-04-19T21:27:20Z

Iiiinnntteerreeeesssssstttiiinnnggg ...

pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) alice
pid-12669 thread-4554166272 (Thread-2) 1492636941.54 loading team
pid-12669 thread-4554166272 (Thread-2) 1492636941.54 old team: n/a
pid-12669 thread-4554166272 (Thread-2) 1492636941.54 slug: 32055ca0-fac1-416b-946b-6938e4f9765f
pid-12669 thread-4554166272 (Thread-2) 1492636941.54 new team: 32055ca0-fac1-416b-946b-6938e4f9765f
pid-12669 thread-4554166272 (Thread-2) 1492636941.54 updated, sleeping ...
pid-12669 thread-4554166272 (Thread-2) 1492636943.55 slept
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) alice
pid-12669 thread-4558372864 (Thread-3) 1492636944.59 loading team
pid-12669 thread-4558372864 (Thread-3) 1492636944.59 old team: n/a
pid-12669 thread-4558372864 (Thread-3) 1492636944.59 slug: 3ac763de-d911-4be6-85eb-85772a23f85b
pid-12669 thread-4558372864 (Thread-3) 1492636944.6 new team: 3ac763de-d911-4be6-85eb-85772a23f85b
pid-12669 thread-4558372864 (Thread-3) 1492636944.6 updated, sleeping ...
pid-12669 thread-4558372864 (Thread-3) 1492636946.6 slept

diff --git a/gratipay/models/package/__init__.py b/gratipay/models/package/__init__.py
index 8ec1ac5..e6963df 100644
--- a/gratipay/models/package/__init__.py
+++ b/gratipay/models/package/__init__.py
@@ -65,6 +65,15 @@ class Package(Model):
     def ensure_team(self, cursor, owner):
         """Given a db cursor and :py:class:`Participant`, insert into ``teams`` if need be.
         """
+
+        #########################################
+        from aspen import log
+        import time
+        for i in range(10):
+            log(owner.username)
+            time.sleep(0.1)
+        #########################################
+
         import time
         from aspen import log as _log

chadwhitacre · 2017-04-19T21:30:14Z

Synchronized:

diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py
index 4275821..20b9cd6 100644
--- a/gratipay/models/participant/email.py
+++ b/gratipay/models/participant/email.py
@@ -283,6 +283,15 @@ class Email(object):
         cursor.run('DELETE FROM claims WHERE nonce=%s', (nonce,))
         package_ids = []
         for package in packages:
+
+            #########################################
+            from aspen import log
+            import time
+            for i in range(10):
+                log()
+                time.sleep(0.1)
+            #########################################
+
             package.ensure_team(cursor, self)
             package_ids.append(package.id)
         self.app.add_event( cursor

chadwhitacre · 2017-04-19T21:30:52Z

Parallel!

diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py
index 4275821..ba9909d 100644
--- a/gratipay/models/participant/email.py
+++ b/gratipay/models/participant/email.py
@@ -243,6 +243,15 @@ class Email(object):
             if (utcnow() - record.verification_start) > EMAIL_HASH_TIMEOUT:
                 return VERIFICATION_EXPIRED
             try:
+
+                #########################################
+                from aspen import log
+                import time
+                for i in range(10):
+                    log(nonce)
+                    time.sleep(0.1)
+                #########################################
+
                 self.finish_package_claims(cursor, nonce, *packages)
                 self.save_email_address(cursor, email)
             except IntegrityError:

chadwhitacre · 2017-04-19T21:32:34Z

Eeeeee! Parallel!

diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py
index 4275821..a8fa11c 100644
--- a/gratipay/models/participant/email.py
+++ b/gratipay/models/participant/email.py
@@ -280,6 +280,15 @@ class Email(object):
     def finish_package_claims(self, cursor, nonce, *packages):
         """Create teams if needed and associate them with the packages.
         """
+
+        #########################################
+        from aspen import log
+        import time
+        for i in range(10):
+            log(nonce)
+            time.sleep(0.1)
+        #########################################
+
         cursor.run('DELETE FROM claims WHERE nonce=%s', (nonce,))
         package_ids = []
         for package in packages:

chadwhitacre · 2017-04-19T21:35:36Z

Parallel, and then synchronized. Wow. A curveball! What is going on? 😳

pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4358180864 (Thread-2) 1492637600.92 loading team
pid-14959 thread-4358180864 (Thread-2) 1492637600.92 old team: n/a
pid-14959 thread-4358180864 (Thread-2) 1492637600.92 slug: ace40d75-7078-4998-8b25-4ece6c8909c2
pid-14959 thread-4358180864 (Thread-2) 1492637600.93 new team: ace40d75-7078-4998-8b25-4ece6c8909c2
pid-14959 thread-4358180864 (Thread-2) 1492637600.93 updated, sleeping ...
pid-14959 thread-4358180864 (Thread-2) 1492637602.93 slept
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) f190faff-d000-476a-9ada-a7eaa8354217
pid-14959 thread-4362387456 (Thread-3) 1492637603.96 loading team
pid-14959 thread-4362387456 (Thread-3) 1492637603.97 old team: n/a
pid-14959 thread-4362387456 (Thread-3) 1492637603.97 slug: 0bab6963-9e36-4267-9599-77d3ad89a8ba
pid-14959 thread-4362387456 (Thread-3) 1492637603.97 new team: 0bab6963-9e36-4267-9599-77d3ad89a8ba
pid-14959 thread-4362387456 (Thread-3) 1492637603.97 updated, sleeping ...
pid-14959 thread-4362387456 (Thread-3) 1492637605.97 slept

diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py
index 4275821..b72daa8 100644
--- a/gratipay/models/participant/email.py
+++ b/gratipay/models/participant/email.py
@@ -280,9 +280,19 @@ class Email(object):
     def finish_package_claims(self, cursor, nonce, *packages):
         """Create teams if needed and associate them with the packages.
         """
+
+        def spew():
+            import aspen, time
+            for i in range(10):
+                aspen.log(nonce)
+                time.sleep(0.1)
+
+        spew()
+
         cursor.run('DELETE FROM claims WHERE nonce=%s', (nonce,))
         package_ids = []
         for package in packages:
+            spew()
             package.ensure_team(cursor, self)
             package_ids.append(package.id)
         self.app.add_event( cursor

chadwhitacre · 2017-04-19T21:38:23Z

Okay! cursor.run, what are you doing?!

diff --git a/gratipay/models/participant/email.py b/gratipay/models/participant/email.py
index 4275821..60dd0fe 100644
--- a/gratipay/models/participant/email.py
+++ b/gratipay/models/participant/email.py
@@ -280,7 +280,15 @@ class Email(object):
     def finish_package_claims(self, cursor, nonce, *packages):
         """Create teams if needed and associate them with the packages.
         """
+
+        def spew(msg):
+            import aspen, time
+            for i in range(10):
+                aspen.log(msg)
+                time.sleep(0.1)
+        spew('parallel')
         cursor.run('DELETE FROM claims WHERE nonce=%s', (nonce,))
+        spew('serial')
         package_ids = []
         for package in packages:
             package.ensure_team(cursor, self)

chadwhitacre · 2017-04-19T21:39:01Z

Sorry, I meant for all of those to go under #4397 (comment). Moving back there ...

dowski · 2017-04-20T23:20:43Z

I'm good with merging this generally, but what about those failures below?

chadwhitacre · 2017-04-21T00:59:20Z

Those are from the commit where I was hacking and slashing to sort out concurrency. Pushed off now (was 8da4f9c).

What about the fact that calling finish_email_verification twice appears to not error out?

dowski · 2017-04-21T02:03:09Z

Ah right - understanding that does seem like a good idea.

chadwhitacre · 2017-04-21T19:48:52Z

Hung up on #4413 ...

chadwhitacre · 2017-04-21T21:18:29Z

Back from #4413. Now how about this concurrency issue ...

chadwhitacre · 2017-04-24T14:36:36Z

Ready again, @dowski.

chadwhitacre · 2017-04-24T14:56:49Z

Rebased to pick up #4414, was 9f9c87e.

dowski

LGTM

Includes: #4335 #4397 #4398 #4404 #4422 #4406 #4410 #4411 #4423

chadwhitacre mentioned this pull request Mar 31, 2017

Integrate initial package claiming PRs #4305

Merged

25 tasks

chadwhitacre force-pushed the project/claim-packages-finish branch 3 times, most recently from 39569db to 42871dd Compare March 31, 2017 20:34

chadwhitacre force-pushed the project/claim-packages-send branch from de1a0a3 to 01c837f Compare April 3, 2017 12:53

chadwhitacre force-pushed the project/claim-packages-finish branch 2 times, most recently from a809a77 to 566c176 Compare April 3, 2017 18:45

chadwhitacre force-pushed the project/claim-packages-finish branch 2 times, most recently from 1f519df to 9cd01ae Compare April 4, 2017 19:01

chadwhitacre added the Review label Apr 4, 2017

chadwhitacre mentioned this pull request Apr 4, 2017

Present claimed packages as projects #4398

Merged

2 tasks

chadwhitacre removed the Review label Apr 14, 2017

chadwhitacre changed the base branch from project/claim-packages-send to project/claim-packages April 17, 2017 11:57

chadwhitacre force-pushed the project/claim-packages-finish branch from 757b498 to 404107c Compare April 17, 2017 12:07

chadwhitacre added the Review label Apr 17, 2017

dowski reviewed Apr 19, 2017

View reviewed changes

chadwhitacre force-pushed the project/claim-packages-finish branch from 8da4f9c to f9a9fcc Compare April 21, 2017 00:58

chadwhitacre mentioned this pull request Apr 21, 2017

Unfrobulate Travis #4413

Closed

chadwhitacre mentioned this pull request Apr 21, 2017

Audit cursor usage for concurrency bugs #4418

Closed

chadwhitacre force-pushed the project/claim-packages-finish branch from 3d8fb44 to 28dffdd Compare April 24, 2017 14:10

chadwhitacre force-pushed the project/claim-packages branch from baf8248 to 521d355 Compare April 24, 2017 14:55

chadwhitacre added 5 commits April 24, 2017 10:56

Implement package claiming

b4267f3

Clean up package.team API a bit

c7f08a8

Update team.package API to match

d041eb7

Failing test for concurrency-safety in claiming

eaa8699

Make finish_email_verification concurrency-safe

1d42094

chadwhitacre force-pushed the project/claim-packages-finish branch from 9f9c87e to 1d42094 Compare April 24, 2017 14:56

chadwhitacre mentioned this pull request Apr 25, 2017

Edit claimed packages #4404

Merged

15 tasks

dowski approved these changes Apr 26, 2017

View reviewed changes

dowski merged commit 4b517cd into project/claim-packages Apr 26, 2017

dowski deleted the project/claim-packages-finish branch April 27, 2017 02:11

chadwhitacre added a commit that referenced this pull request Apr 28, 2017

Checkpoint 2

771c40f

Includes: #4335 #4397 #4398 #4404 #4422 #4406 #4410 #4411 #4423

chadwhitacre added a commit that referenced this pull request Apr 28, 2017

Checkpoint 2

eb5921f

Includes: #4335 #4397 #4398 #4404 #4422 #4406 #4410 #4411 #4423

chadwhitacre added a commit that referenced this pull request May 5, 2017

Checkpoint 2

9898a38

Includes: #4335 #4397 #4398 #4404 #4422 #4406 #4410 #4411 #4423

chadwhitacre added a commit that referenced this pull request May 9, 2017

Checkpoint 2

24559cb

Includes: #4335 #4397 #4398 #4404 #4422 #4406 #4410 #4411 #4423

		return cursor.one('SELECT t.*::teams FROM teams t WHERE t.id=%s', (self.team_id,))


		def get_or_create_linked_team(self, cursor, owner):

Receive package claim link-back #4397

Receive package claim link-back #4397

Conversation

chadwhitacre commented Mar 31, 2017 • edited Loading

Requirements

Todo

chadwhitacre commented Mar 31, 2017

chadwhitacre commented Mar 31, 2017

chadwhitacre commented Mar 31, 2017

chadwhitacre commented Apr 3, 2017

Todo

chadwhitacre commented Apr 3, 2017

chadwhitacre commented Apr 14, 2017

chadwhitacre commented Apr 17, 2017

dowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chadwhitacre Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chadwhitacre Apr 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chadwhitacre commented Apr 19, 2017

chadwhitacre commented Apr 19, 2017

chadwhitacre commented Apr 19, 2017

chadwhitacre commented Apr 19, 2017

chadwhitacre commented Apr 19, 2017

chadwhitacre commented Apr 19, 2017

chadwhitacre commented Apr 19, 2017 • edited Loading

dowski commented Apr 20, 2017 • edited Loading

chadwhitacre commented Apr 21, 2017 • edited Loading

dowski commented Apr 21, 2017

chadwhitacre commented Apr 21, 2017

chadwhitacre commented Apr 21, 2017

chadwhitacre commented Apr 24, 2017

chadwhitacre commented Apr 24, 2017

dowski left a comment

Choose a reason for hiding this comment

chadwhitacre commented Mar 31, 2017 •

edited

Loading

chadwhitacre Apr 19, 2017 •

edited

Loading

chadwhitacre Apr 19, 2017 •

edited

Loading

chadwhitacre commented Apr 19, 2017 •

edited

Loading

dowski commented Apr 20, 2017 •

edited

Loading

chadwhitacre commented Apr 21, 2017 •

edited

Loading