nchah / github-traffic-stats

Get statistics on web traffic to your GitHub repositories.
https://pypi.org/project/github-traffic-stats/
121 stars 31 forks source link

Add support for username:password pairs #10

Closed sygibson closed 6 years ago

sygibson commented 6 years ago

This Pull adds support to specify username:password for authentication.

If only username is specified (eg no :password), then the interactive dialog will be called as per the original usage.

The password can be usernames Github account password, or a Github Access Token, which is documented at: https://help.github.com/articles/creating-a-personal-access-token-for-the-command-line/

nchah commented 6 years ago

Hi Shane! Thank you for the pull request and for updating the documentation! This should be merged soon.

I realize this may be an edge case, but GitHub also accepts "spaces" in passwords. With that in mind, perhaps it's better to remove the ".strip" call from line 232 and 238? Hi Shane! Thank you for the pull request and for updating the documentation! This should be merged soon.

I realize this may be an edge case, but GitHub also accepts "spaces" in passwords. With that in mind, perhaps it's better to remove the ".strip" call from line 232 and 238?

sygibson commented 6 years ago

That's a good question. I've seen a lot of auto-complete form responses in browsers inject a trailing space, assuming another word is next. That is the use case I was thinking of with the .strip call. You're right that it might interfere with input if the user has a legitimate space on the beginning or end of the user/pass pair.

nchah commented 6 years ago

Thanks for the explanation! I am also a fan of sanitizing for spaces. If users have trailing/leading spaces in their password, they could always input it themselves under the usual input method (i.e. $ python main.py 'nchah' '[repo...]') or someone may file an issue/PR in the future as needed.

AnthonyBloomer commented 6 years ago

Nice contribution Shane. I'll get this pushed to PyPi at some stage today.