implement HTTPS support for cloning (#225)

[matthiasbalke]

I finally found some time to implement this. I took @flaviouk idea and added a compatible argument parsing around it. This is my first python project so I would love to get honest feedback, about parts that can be improved.

How do you like the idea of the mutally exclusive argument group of https vs. ssh?

Changes

• added configuration for HTTP(S) support
• added / configured cacerts nix package to allow SSL validation
• updated usage in Readme with latest --help output
• addded help section in Readme for configuring HTTP(S)

[matthiasbalke]

@jcpetruzza @aschmolck, @jasontrost I fixed some linting issues. Now the PR is ready to review.

[nejch]

@tclh123 this PR makes a lot of sense to avoid providing duplicate credentials. Most bots work with access tokens these days and they can be easier to rotate/provision. I'm looking at using this on our CE instance and would love to see it merged. Do you need some help with this repo to catch up on some of the PRs? I see some have been open for a while (https://github.com/smarkets/marge-bot/pull/254 looks very useful too).

My only comment @matthiasbalke is some of the copied code in HttpsRepoManager could maybe be deduplicated in a base class or the common methods put in a Mixin in case it grows later:

class RepoMixin:
    def forget_repo(self, project):
        self._repos.pop(project.id, None)

    @property
    def user(self):
        return self._user

    @property
    def root_dir(self):
        return self._root_dir

class SshRepoManager(RepoMixin):
# ...

class HttpsRepoManager(RepoMixin):
# ...

[matthiasbalke]

Thanks for your feedback @nejch. I wasn't sure how to extract the common parts into a base class, as I'm new to class hierarchies in python. I'm happy to refactor this part as you suggested.

I'm also wondering if the maintainers need some help, as it seems to be a bit quite in here and the project is really usefull to us and probably to a lot more people.

[nejch]

Thanks for your feedback @nejch. I wasn't sure how to extract the common parts into a base class, as I'm new to class hierarchies in python. I'm happy to refactor this part as you suggested.

I'm also wondering if the maintainers need some help, as it seems to be a bit quite in here and the project is really usefull to us and probably to a lot more people.

Maybe a base class makes sense as well with a shared init. Or also just one manager with _get_via_ssh() and _get_via_https() internal methods based on the arguments provided. Whatever works, I dont like writing classes either :D

[matthiasbalke]

@nejch I'm not sure how to share the __init__ function. I've tried it like this, but the syntax is wrong. Can you give me a hint on how to call the RepoManagers __init__ function from the implementing classes?

class RepoManager:

    def __init__(self, user, root_dir, timeout=None, reference=None):
        self._root_dir = root_dir
        self._user = user
        self._repos = {}
        self._timeout = timeout
        self._reference = reference

class SshRepoManager(RepoManager):

    def __init__(self, user, root_dir, ssh_key_file=None, timeout=None, reference=None):
        super(RepoManager, self).__init__(self, user, root_dir, timeout, reference)
        self._ssh_key_file = ssh_key_file

class HttpsRepoManager(RepoManager):

    def __init__(self, user, root_dir, auth_token=None, timeout=None, reference=None):
        super(RepoManager, self).__init__(self, user, root_dir, timeout, reference)
        self._auth_token = auth_token

[nejch]

@matthiasbalke I think your exact example is described here in this answer: https://stackoverflow.com/a/39887759

[matthiasbalke]

@nejch I've extracted the common code. Thanks to your example.

@tclh123 As you pushed the last release tag, I asume you are one of the maintainers. Are you willing to merge this? If there is anything you dislike about my changes please let me know.

[qqshfox]

Hi @matthiasbalke , thank you for the contribution! I believe this feature shall be able to give developers more flexibility to handle different access methods to repositories and managing their credentials. I'm very happy to see this merged. Would you mind resolving the conflicts and squashing them into a single commit? Thanks.

[matthiasbalke]

@qqshfox do you have any idea, why my PR branch is not building on CI? The request log states user has not confirmed their email address, but I don't know how to validate my email.

[qqshfox]

unsure why CI doesn't build. will investigate. suspecting travis-ci.org might not take new builds because it'll be shutting down soon.

Please be aware travis-ci.org will be shutting down in several weeks, with all accounts migrating to travis-ci.com.

[nejch]

Looks like you should be able to run this now if you rebase @matthiasbalke :)

[matthiasbalke]

@qqshfox I squashed the commits. I hope everything is now ready to be merged.

[qqshfox]

LGTM. thank you @matthiasbalke !

[matthiasbalke]

Thanks for merging it!