Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Implement a check for whether we posted MassPay #4256

Merged
merged 3 commits into from
Feb 17, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Dec 23, 2016

Hopefully will prevent recurrences of gratipay/inside.gratipay.com#951 (comment).

@chadwhitacre chadwhitacre changed the title Implement a check for whether we posted masspay Implement a check for whether we posted MassPay Dec 23, 2016
@chadwhitacre
Copy link
Contributor Author

Build is failing on #4236.

@chadwhitacre chadwhitacre changed the base branch from master to fix-iter-payday-events-test December 29, 2016 22:11
@chadwhitacre
Copy link
Contributor Author

Base changed to #4236, builds restarted.

@chadwhitacre chadwhitacre force-pushed the fix-iter-payday-events-test branch from cfa0d32 to e5431c3 Compare January 6, 2017 18:05
@mattbk
Copy link
Contributor

mattbk commented Jan 24, 2017

#4236 is merged, should base be changed back to master?

@chadwhitacre chadwhitacre changed the base branch from fix-iter-payday-events-test to master January 24, 2017 12:07
@chadwhitacre
Copy link
Contributor Author

Yup! Done and rebased. Previous was 20156d0.

@mattbk
Copy link
Contributor

mattbk commented Jan 26, 2017

This looks okay to me as far as I can follow it. @nobodxbodon?

@chadwhitacre
Copy link
Contributor Author

Rebased again.

@chadwhitacre chadwhitacre force-pushed the check-for-posted-masspay branch from d7425a7 to 80578b8 Compare January 26, 2017 17:15
JOIN exchange_routes er
ON e.route = er.id
WHERE er.network = 'paypal'
AND e.amount < 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's this mean?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantics of the exchanges.amount field are somewhat goofy (especially as it interacts with the fee field; see #2442), but fundamentally a negative amount indicates a payout, whereas a positive amount indicates a payin.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining. Shall we document this in schema.sql (not necessary in this PR)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bin/masspay.py Outdated
@@ -193,6 +193,11 @@ def post_back_to_gratipay():
except KeyError:
gratipay_base_url = 'https://gratipay.com'

nmasspays = int(requests.get(gratipay_base_url + '/dashboard/nmasspays').text())
if nmasspays < 10:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so this assumes there should be more than 10 teams to pay to? any way to override this without changing code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would we want to override it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In case that there was no more than 10 teams to pay out to. Or is that impossible?

Copy link
Contributor Author

@chadwhitacre chadwhitacre Feb 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm ... theoretically, I suppose. We haven't been below 10 in a long time that I can recall (famous last words). This is the test I came up with to decide whether masspay had been run the previous week or not. I would say in the case where it was run but there were less than 10 payouts, the person running payday could adapt to the situation by manually modifying the script before rerunning. That should be a rare occurrence.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. Would it be helpful to add some instructions in the print out below, like "if this is a false alarm, please modify xxx manually and rerun"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about 2de1d66?

@chadwhitacre chadwhitacre force-pushed the check-for-posted-masspay branch from 2de1d66 to 45c791f Compare February 9, 2017 12:42
@chadwhitacre
Copy link
Contributor Author

Rebased, was 2de1d66.

Copy link
Contributor

@nobodxbodon nobodxbodon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@chadwhitacre chadwhitacre force-pushed the check-for-posted-masspay branch from 45c791f to e5fd92b Compare February 17, 2017 12:35
@chadwhitacre
Copy link
Contributor Author

Rebased, was 45c791f. Ready to merge!

@mattbk mattbk merged commit 85d767f into master Feb 17, 2017
@mattbk mattbk deleted the check-for-posted-masspay branch February 17, 2017 15:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants