RF+BF: github - remove user/password authentication, use only tokens #5218
Conversation
- minor __doc__ tuneup for typos, and consistent GitHub (not Github) - also removes storing of a (now "entered", not generated) token into git configuration. If desired to do so - do manually. To centralize entered credentials handling, regular credential store is used - VCR tapes were updated (with token manually replaced after) for two tests and 3rd commented out was removed completely - in some cases a little better error reporting now (e.g., giving a hint that token likely lack permissions to find a repository while creating ;)) Overall, it is not yet "much better" but it should be working and usable again.
yarikoptic
added
the
merge-if-ok
|
I have adjusted the handbook section that walks through |
test__make_github_repos() is failing on Travis because the the logged
BadCredentialsException ends with
very bad status "some data"
instead of
very bad status some data
I see the same failure locally (PyGithub 1.53 and Python 3.7.3). I'm
hesitant to change it though, because presumably this passed on
@yarikoptic's local machine.
The exact form isn't the crucial part of this test, so just chop off
the trailing part of the query string.
https://travis-ci.com/github/datalad/datalad/jobs/456528059#L2319
Codecov Report
@@ Coverage Diff @@
## maint #5218 +/- ##
==========================================
- Coverage 89.90% 89.90% -0.01%
==========================================
Files 294 294
Lines 40954 40919 -35
==========================================
- Hits 36820 36787 -33
+ Misses 4134 4132 -2
Continue to review full report at Codecov.
|
|
Thank you @adswa for fixups -- FWIW, they look fine to me. |
|
@yarikoptic Are you fine with the change in aed2886 (TST: github: Loosen string matching of credential exception, 2020-12-09)? Or would you like to verify that the original test passed on your end (i.e. you're seeing different results than what happens on Travis and my local machine)? |
|
well, I have given my blessing the changes. I think it would have been nice to keep it a bit more strict, even if just to keep the beginning of the exception string, but I was ok as is with @adswa 's changes... well -- since you are asking, I will add back at least the |
You said: "Thank you @adswa for fixups -- FWIW, they look fine to me." So, it's not clear that you're considering aed2886, which is a commit from me. Hence why I asked explicitly.
That part was dropped because it fails on Travis and for me locally. (edit: never mind, you're talking about just the leading part; ok). As aed2886 said: |
|
ha ha -- and I was thinking "Adina gave such a nice commit message, like Kyle does, good for her" ;) I have missed the fact that it was commit from you @kyleam not Adina ;) |
|
@yarikoptic Just out of curiosity, do you think the difference in that output is due to the PyGithub version? |
|
I do not know and not care much enough ;) FWIW locally I have 1.43.7-1 from debian's python3-github |
That's fair, but I hate unsolved mysteries :)
Thanks. Indeed this change comes with PyGithub's v1.43.8-12-ga0f01cf9 (Remove more Python version specific code, 2019-08-28). |
git configuration. If desired to do so - do manually. To
centralize entered credentials handling, regular credential store is
used
tests and 3rd commented out was removed completely (Closes Reenable github integration test #4079)
hint that token likely lack permissions to find a repository while
creating ;))
Overall, it is not yet "much better" but it should be working and
usable again.
If merged, closes #3947