Skip to content
This repository was archived by the owner on Apr 29, 2022. It is now read-only.

Conversation

@sm6xmm
Copy link
Contributor

@sm6xmm sm6xmm commented Feb 8, 2020

Perhaps not the most beautiful code, but it does appear work.

@sm6xmm
Copy link
Contributor Author

sm6xmm commented Feb 8, 2020

Hmm, those tests passed locally. I don't have time to look at it for a few days, can someone else have a look?

@malemburg
Copy link
Member

There seems to be an issue with the test setup:

stripe.error.AuthenticationError: You did not provide an API key. You need to provide your API key in the Authorization header, using Bearer auth (e.g. 'Authorization: Bearer YOUR_SECRET_KEY'). See https://stripe.com/docs/api#authentication for details, or we can help at https://support.stripe.com/.

FWIW: It's not a good idea to put such keys into the repo, but perhaps there's some other way to get the test keys to the test runner. In Gitlab, you can setup CI env vars for this purpose.

@sm6xmm sm6xmm requested review from artcz and umgelurgel February 8, 2020 11:25
@sm6xmm
Copy link
Contributor Author

sm6xmm commented Feb 8, 2020

There seems to be an issue with the test setup:

stripe.error.AuthenticationError: You did not provide an API key. You need to provide your API key in the Authorization header, using Bearer auth (e.g. 'Authorization: Bearer YOUR_SECRET_KEY'). See https://stripe.com/docs/api#authentication for details, or we can help at https://support.stripe.com/.

FWIW: It's not a good idea to put such keys into the repo, but perhaps there's some other way to get the test keys to the test runner. In Gitlab, you can setup CI env vars for this purpose.

Aha, there are keys in the repo already (generic stripe keys I think, because they are not our test keys), so try it with the keys in pycon/dev_settings.py. I'll telegram you our test keys in case those don't work.

@umgelurgel
Copy link
Contributor

Thanks for delivering this! Looks great - I've tested manually and can't reproduce the test failures.

I'll take over fixing CI (going to mark the test as xfailed to get this merged first) and I'll apply any review feedback I'd have to get this merged sooner rather than later. Let me know if you have any objections :)

@umgelurgel
Copy link
Contributor

Closing in favour of #1180, as I can't disable the tests in a branch in a forked repo.

@umgelurgel umgelurgel closed this Feb 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants