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

stop computing an email for balanced #453

Closed
chadwhitacre opened this issue Jan 9, 2013 · 22 comments
Closed

stop computing an email for balanced #453

chadwhitacre opened this issue Jan 9, 2013 · 22 comments
Labels

Comments

@chadwhitacre
Copy link
Contributor

The Balanced API used to require an email address, so we compute one from the Gittip username + "@gittip.com". I believe Balanced no longer requires this, so we should stop doing it. It tripped us up on #451.

@ArcTanSusan
Copy link
Contributor

Should be changes all be in billing/__init__.py? Is there a script for running the tests on billing/?

@chadwhitacre
Copy link
Contributor Author

@onceuponatimeforever There is a script for running the tests. The simplest way is with:

$ make test

Do you have a local Postgres set up? The tests are really slow if you use Postgression.

@ArcTanSusan
Copy link
Contributor

I'm using the postgres app for MacOS for running postgres. What should be the expected output of make test? After following the configuration rules for testing, I'm getting errors (again) when I run make test. See stacktrace below:

[gittip] Susans-MacBook-Air:www.gittip.com susantan$ make test ./env/bin/swaddle tests/env ./recreate-schema.sh ./recreate-schema.sh: line 13: export:DATABASE_URL,': not a valid identifier
./recreate-schema.sh: line 13: export: default_tests.env': not a valid identifier ./recreate-schema.sh: line 13: export:something.': not a valid identifier
make: *** [test-schema] Error 1
[gittip] Susans-MacBook-Air:www.gittip.com susantan$ cd tests/
[gittip] Susans-MacBook-Air:tests susantan$ swaddle env ../env/bin/nosetests
[SWADDLE] Command ../env/bin/nosetests does not exist; exiting.
`

@seanlinsley
Copy link
Contributor

You need to update your DATABASE_URL in local.env. Right now it has an error string from Postgression instead of a proper database URL

@ArcTanSusan
Copy link
Contributor

I have postgres running now. Then, 'make schema' gives the following error stack trace:

[gittip] Susans-MacBook-Air:www.gittip.com susantan$ make schema ./env/bin/swaddle local.env ./recreate-schema.sh Recreating public schema ... psql: could not connect to server: No route to host Is the server running on host "5432" (0.0.21.56) and accepting TCP/IP connections on port ? make: *** [schema] Error 2

@chadwhitacre
Copy link
Contributor Author

Is the server running on host "5432" (0.0.21.56) and accepting TCP/IP connections on port ?

That makes me think you have a malformed DATABASE_URL. Mind pasting that?

@ghost ghost assigned zbynekwinkler Oct 12, 2013
@zbynekwinkler
Copy link
Contributor

From looking at the code it seams that we would just skip this search:
https://github.com/gittip/www.gittip.com/blob/master/gittip/billing/__init__.py#L44
Right? If no uri is supplied, we just create a new Account (?)
BTW: current balanced api docs have Customer instead of Account so it is a bit harder to orient.

@zbynekwinkler
Copy link
Contributor

But it might not be worth touching this given #1556 since there is no risk leaving this in, is there?

@zbynekwinkler
Copy link
Contributor

https://docs.balancedpayments.com/current/api.html?language=python#creating-an-account

Accounts have been deprecated. Please use Customers instead.

@zbynekwinkler
Copy link
Contributor

@chadwhitacre
Copy link
Contributor Author

@zwn Should we reticket the change from Account to Customer?

@zbynekwinkler
Copy link
Contributor

@whit537 That depends. I am not sure what "deprecated" in balanced terms means. I wouldn't do it unless there is a clear added value in it for us. From reading the description it is needed when you want to do a payout to an unverified customer, which was not possible with accounts. Our workflow is settled, no one complains, so for me no, lets not do it (the change from Account to Customer).

Does balanced do some upgrades to the api because of security issues or something else that would force us to do it?

@chadwhitacre
Copy link
Contributor Author

Does balanced do some upgrades to the api because of security issues or something else that would force us to do it?

Not that I'm aware of. However, I think there can be value in staying current with evolving dependencies. It's easier to do periodic smaller upgrades than have to upgrade all at once when some crisis hits. No?

@zbynekwinkler
Copy link
Contributor

It's easier to do periodic smaller upgrades than have to upgrade all at once when some crisis hits. No?

😄 Could be. But there is always the YAGNI principle. So I guess it really depends on the number and difficulty of the "small" upgrades versus the times when some crisis hits. But given that we are currently resource constrained I'd try to minimize current, not future, work.

@chadwhitacre
Copy link
Contributor Author

Fair enough. :-)

@zbynekwinkler
Copy link
Contributor

I have removed this issue from the Infrastructure milestone. It would be nice to have this cleaned up but it is not something that holds us back very much.

@chadwhitacre
Copy link
Contributor Author

We have a user, https://www.gittip.com/Voynov%20E./, who tried to add a credit card, and ran afoul of this issue because Balanced rejected the email we computed (Voynov [email protected]). Apart from the fact that this user looks quite suspicious, we should still fix this bug.

Here's the sentry entry:

https://app.getsentry.com/gittip/gittip/group/14791500/events/760258656/

@zbynekwinkler
Copy link
Contributor

Where "this bug" is...?

@chadwhitacre
Copy link
Contributor Author

@zwn Did you follow the sentry link?

@zbynekwinkler
Copy link
Contributor

@whit537 Yes. Balanced is complaining that a valid email address is not valid. I was not sure what "this bug" refers to. Is it:
a) allowing space in usernames in gittip
b) encoding the space as %20 into email
c) using the email with balanced (this ticket)
d) something else I didn't think of

@chadwhitacre
Copy link
Contributor Author

Answer: (c).

Since we already have this ticket open and fixing this ticket would also address the specific case we just hit, I decided to associate that specific case with this ticket. Do you think that's the wrong approach?

@Changaco
Copy link
Contributor

Changaco commented Sep 7, 2014

Closing as obsolete.

@Changaco Changaco closed this as completed Sep 7, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants