🚧 Add Gogs support #82

pyhedgehog · 2016-12-20T07:26:38Z

Current status:

Implemented:
- get_auth_token accepts url of profile page as login
- connect
- create
- delete
- list
Tested:
- config
- list and list -l
- open
- add
- create
- delete
- clone
Added custom config options: (see GoGSService.connect)
- verify - yes/no or path to ca-bundle
- default-private - set to yes if new repositories should be private
- ssh-url - i.e. 127.0.0.1:3022
Uses proxy from git config
Unable to implement as unsupported by Gogs API:
- Gists.
- Pull requests.
- Fork unavailable via API
get_auth_token can't return alternative fqdn and other custom options back to config command.

resolves #18

guyzmo · 2016-12-20T11:52:22Z

git_repo/services/ext/gogs.py

+        if verify.lower().strip() in ('0','no','false',''):
+            verify = False
+        elif verify.lower().strip() in ('1','yes','true'):
+            verify = True


This feature is being implemented in another pull request, which is ready and will be released along the next release (which is due relatively soon, it's mostly depending on Gitlab to merge a patch I made).

So I'm merging that PR into devel (I actually thought I already did), so you can rebase your branch and use self.insecure instead of verify.

guyzmo · 2016-12-20T11:55:10Z

git_repo/services/ext/gogs.py

+        elif verify.lower().strip() in ('1','yes','true'):
+            verify = True
+        self.default_private = self.config.get('default-private', 'true').lower() not in ('0','no','false')
+        self.ssh_url = self.config.get('ssh-url', None) or self.fqdn


is it likely that ssh_url would be different than based on fqdn? The assumption I'm making with self-hosted gitlab is that the fqdn for both ssh and http is the same, so for homogeinity I'd rather do the same here (and if it's wrong, then maybe make a fix for both gogs and gitlab), but I'm open to counter arguments on it.

And BTW the way you're using it, it's actually ssh_fqdn as an URL is supposed to look like ssh:https:// and might contain a path.

ssh-url can contains either fqdn with non-standard port 127.0.0.1:3022, or "fqdn" that refers some alias in ~/.ssh/config with Port and other parameters (i.e. ProxyCommand) pre-customized.

hm… ok, though it's not the right place to configure that, it should be done the same way self.insecure is implemented, so all services can access that (as both gitlab and github can run on private instances).

guyzmo · 2016-12-20T11:58:36Z

git_repo/services/ext/gogs.py

+            if proxy:
+                proxies[scheme] = proxy
+        try:
+            self.session = requests.Session()


The session shall be created in __init__, ideally when instanciating the external library that abstract the API. The reason for that is that the tests rely on the session instance to be monkey patched in order to be recorded and replayed.

connect() is called from git_repo.services.service.RepositoryService.__init__, isn't it?

not necessarily, only if a repository argument is given to the __init__(). Basically, the tests framework is designed on the fact that instanciation of the requests.Session() (and any abstraction in between) is setup in __init__() whereas the connection is setup in connect(), making it then possible to hijack the Session instance in between.

guyzmo · 2016-12-20T11:59:34Z

git_repo/services/ext/gogs.py

+            if not verify:
+                try:
+                    import urllib3
+                    urllib3.disable_warnings()


why are you disabling urllib3 warnings? If it needs to yell, let it yell! ☺

guyzmo · 2016-12-20T12:00:12Z

git_repo/services/ext/gogs.py

+                if not self._privatekey:
+                    raise ConnectionError('Could not connect to GoGS. '
+                                          'Please configure .gitconfig '
+                                          'with your github private key.') from err


s/github/GoGS/

☺

guyzmo · 2016-12-20T12:16:50Z

git_repo/services/ext/gogs.py

+            verify = False
+        elif verify.lower().strip() in ('1','yes','true'):
+            verify = True
+        self.default_private = self.config.get('default-private', 'true').lower() not in ('0','no','false')


I'm not for a config option for that, which would then be not homogeneous with the other services. Though you're making a good point here, we're missing private status of a repository creation in the API, so I believe we should add a --private argument to both create and fork.

About the configurations (and setting up defaults), there's issue #46 discussing this, which aims at doing things in a smart way, until then adding CLI parameters shouldn't be too much of a pain. So, I believe a good time to think about that would be for 2.0!

guyzmo · 2016-12-20T12:20:45Z

git_repo/services/ext/gogs.py

+        for scheme in 'http https'.split():
+            proxy = config.get_value(scheme, 'proxy', '')
+            if proxy:
+                proxies[scheme] = proxy


proxies support is a great idea I totally did not think of…

but again, it should be implemented outside of the service implementation, so that it's accessible to all services (the same way self.insecure is implemented).

so either you remove it, and make an issue/PR to add this globally, or you leave it, and we'll replace that when we'll implement proxy support across services. Not sure what's the smartest.

opened #105 about that.

guyzmo · 2016-12-20T12:36:14Z

ok, so first of all I'm stumped that there's no API for forking a project! It's like one of the most basic feature to be expected from such a service 😖 And I hope that requests will land soon… BTW requests alone without a git ref that can be fetched is only half the feature…

That being said, it's a great job you've done 👍
but I'm not found of having everything as raw requests implemented in git-repo, because we're loosing the level of abstraction that an intermediate library provides.

I prefer to work on with a library that can gather more efforts from different projects implementing it to make it right, and eventually to maintain it when the remote API changes. And, then the RepositoryService specialization is just integrating that library for the special use cases of the project.

At the very least, if you want to roll your own API implementation (which is pretty clean overall), I'd then ask you to move it all within its own class:

class GogsAPI:
    def __init__(self):
        self.session = requests.Session()
        …

    def get_user(self, …):
        …

    def create(self):
        …

    def delete(self):
        …

    def iter_repositories(self):
        …

and then use that within GoGSService. Eventually, we might consider extracting that either to its own module in git-repo, or to a new library python-gogs.

pyhedgehog · 2016-12-20T15:11:36Z

Ok. Let's close this one and I'll implement it second time from scratch.
Also I'm going to attempt to create pull requests for most important features for gogs and python-gogs-client.

guyzmo · 2016-12-20T15:25:47Z

you can actually keep that PR and squash the merges (git push -f) to overwrite your changes. Then you can make a backup of your existing changes as another branch on your own fork just for the sake of it.

guyzmo · 2017-02-03T01:42:49Z

(actually merged)

pyhedgehog changed the title ~~#18: Gogs support~~ GH18: Gogs support Dec 20, 2016

pyhedgehog mentioned this pull request Dec 20, 2016

Add gogs support #18

Closed

guyzmo assigned pyhedgehog Dec 20, 2016

guyzmo self-requested a review December 20, 2016 11:33

guyzmo added the in progress label Dec 20, 2016

guyzmo reviewed Dec 20, 2016

View reviewed changes

guyzmo closed this Dec 20, 2016

guyzmo removed the in progress label Dec 20, 2016

guyzmo reopened this Dec 20, 2016

guyzmo added backlog in progress and removed backlog labels Dec 20, 2016

pyhedgehog closed this Dec 20, 2016

guyzmo removed the in progress label Dec 20, 2016

guyzmo reopened this Dec 20, 2016

guyzmo added backlog in progress and removed backlog labels Dec 20, 2016

guyzmo changed the title ~~GH18: Gogs support~~ 🚧 Add Gogs support Dec 21, 2016

pyhedgehog mentioned this pull request Dec 21, 2016

Automatic Repositories gogs/gogs#1062

Open

guyzmo added this to the 1.10 milestone Dec 24, 2016

pyhedgehog force-pushed the features/gogs branch from ab2f09a to a017ffd Compare January 21, 2017 14:55

Gogs support (second attempt uses gogs_client and includes tests)

de317f3

pyhedgehog force-pushed the features/gogs branch from a017ffd to de317f3 Compare January 21, 2017 15:03

guyzmo added ready and removed in progress labels Feb 3, 2017

guyzmo closed this Feb 3, 2017

guyzmo removed the ready label Feb 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🚧 Add Gogs support #82

🚧 Add Gogs support #82

pyhedgehog commented Dec 20, 2016 •

edited by guyzmo

Loading

guyzmo Dec 20, 2016

guyzmo Dec 20, 2016

pyhedgehog Dec 20, 2016

guyzmo Dec 20, 2016

guyzmo Dec 20, 2016

pyhedgehog Dec 20, 2016

guyzmo Dec 20, 2016

guyzmo Dec 20, 2016

guyzmo Dec 20, 2016 •

edited

Loading

guyzmo Dec 20, 2016

guyzmo Dec 20, 2016

guyzmo Jan 22, 2017

guyzmo commented Dec 20, 2016

pyhedgehog commented Dec 20, 2016

guyzmo commented Dec 20, 2016

guyzmo commented Feb 3, 2017

🚧 Add Gogs support #82

🚧 Add Gogs support #82

Conversation

pyhedgehog commented Dec 20, 2016 • edited by guyzmo Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guyzmo Dec 20, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

guyzmo commented Dec 20, 2016

pyhedgehog commented Dec 20, 2016

guyzmo commented Dec 20, 2016

guyzmo commented Feb 3, 2017

pyhedgehog commented Dec 20, 2016 •

edited by guyzmo

Loading

guyzmo Dec 20, 2016 •

edited

Loading