Add API for token management #2

pyhedgehog · 2016-12-22T03:33:21Z

New class gogs_client.entries.TokenInfo.
New method gogs_client.interface.get_tokens() - returns list of TokenInfo.
New method gogs_client.interface.create_token() - returns new TokenInfo.
New method gogs_client.interface.ensure_token() - returns new or old auth.Token.

coveralls · 2016-12-22T12:37:35Z

Coverage increased (+0.4%) to 92.857% when pulling 74faf4d on pyhedgehog:feature/token into 8ca42a1 on unfoldingWord-dev:master.

ethantkoenig · 2016-12-22T15:24:45Z

gogs_client/interface.py

+        """
+        Returns tokens defined for specified user.
+        If no user specified uses user authenticated by the given authentication.
+        Right now, authentication must be UsernamePassword (not Token).


Why must auth be UsernamePassword? self.authenticated_user(..) and self._get(..) should be able to handle any authentication type.

It's server requirement:
"Create a access token requires that you are authenticated via basic authentication"
routers/api/v1/api.go:190

ethantkoenig · 2016-12-22T15:26:15Z

gogs_client/interface.py

+        """
+        Creates new token if token with specified name for specified user does not exists.
+        If no user specified uses user authenticated by the given authentication.
+        Right now, authentication must be UsernamePassword (not Token).


Ditto, why?

Authentication must be UsernamePassword is server requirement.

ethantkoenig · 2016-12-22T15:52:25Z

gogs_client/interface.py



 class GogsApi(object):
    """
-    A Gogs client, serving as a wrapper around the Gogs HTTP API.
+    A Gogs API entries, serving as a wrapper around the Gogs HTTP API.


Why does this need to change? API entries is a little misleading, since there isn't a one-to-one correspondence between these functions and the Gogs API endpoints.

~~Because this is not consistent client with preset access configuration and persistent session (keep-alive and so on). This is collection of unrelated API wrappers.~~
Sorry. I've reverted this accidental change.

ethantkoenig · 2016-12-22T15:56:06Z

gogs_client/entities.py

@@ -240,3 +240,46 @@ def pull(self):
            :rtype: bool
            """
            return self._pull
+
+
+class TokenInfo(object):


Does this need to be a separate class? Can we just add a name property to the existing Token class?

coveralls · 2016-12-22T21:01:28Z

Coverage increased (+0.4%) to 92.771% when pulling 74075fb on pyhedgehog:feature/token into 8ca42a1 on unfoldingWord-dev:master.

coveralls · 2016-12-22T21:10:35Z

Coverage increased (+0.4%) to 92.771% when pulling 68c76e9 on pyhedgehog:feature/token into 8ca42a1 on unfoldingWord-dev:master.

ethantkoenig · 2016-12-23T04:11:42Z

gogs_client/entities.py

@@ -242,14 +243,14 @@ def pull(self):
            return self._pull


-class TokenInfo(object):
+class TokenInfo(Token):


Could we just add a new property (name) to the existing Token class, instead of introducing a new subclass? I think it would be best to keep the class hierarchies simple, and don't see a pressing need for a subclass here.

ethantkoenig · 2016-12-23T04:24:50Z

gogs_client/interface.py

+        :param str username: username of owner of new token
+
+        :return: token authenticator
+        :rtype: Token


As things currently stand, shouldn't this be TokenInfo? For instance, this method could return self.create_token(..), which is of type TokenInfo.

I still would suggest scraping the TokenInfo class altogether (see other comment), but want to make sure I'm not missing something.

As far as I understand your auth.Token should not be bound to json parsing.

I don't see what's wrong with giving auth.Token a from_json(..) method.

coveralls · 2016-12-23T21:31:35Z

Coverage increased (+0.4%) to 92.793% when pulling 930246e on pyhedgehog:feature/token into 8ca42a1 on unfoldingWord-dev:master.

pyhedgehog · 2016-12-24T17:40:16Z

@ethantkoenig, am I right that all questions are answered and all requests fulfilled?

ethantkoenig · 2016-12-24T18:45:51Z

@pyhedgehog I would still like to combine Token and TokenInfo. Repeating my earlier comment, I don't see what's wrong with giving auth.Token a from_json(..) method (which you cited as a reason for not combining them).

coveralls · 2016-12-24T20:39:15Z

Coverage increased (+0.3%) to 92.749% when pulling ea139fe on pyhedgehog:feature/token into 8ca42a1 on unfoldingWord-dev:master.

pyhedgehog · 2016-12-24T20:42:09Z

@ethantkoenig, Merged.

ethantkoenig · 2016-12-24T20:44:21Z

@pyhedgehog LGTM. Rebase down to a single commit, and I'll merge. Thanks for the contribution!

coveralls · 2016-12-24T21:04:19Z

Coverage increased (+0.3%) to 92.749% when pulling 65e82cf on pyhedgehog:feature/token into da455d1 on unfoldingWord-dev:master.

pyhedgehog · 2016-12-24T21:07:42Z

@ethantkoenig, Done.

ethantkoenig suggested changes Dec 22, 2016

View reviewed changes

guyzmo mentioned this pull request Dec 23, 2016

Add gogs support guyzmo/git-repo#18

Closed

ethantkoenig suggested changes Dec 23, 2016

View reviewed changes

ethantkoenig approved these changes Dec 24, 2016

View reviewed changes

Add API for token management

65e82cf

pyhedgehog force-pushed the feature/token branch from ea139fe to 65e82cf Compare December 24, 2016 21:03

ethantkoenig merged commit 8fa5efb into unfoldingWord-dev:master Dec 24, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add API for token management #2

Add API for token management #2

pyhedgehog commented Dec 22, 2016

coveralls commented Dec 22, 2016 •

edited

Loading

ethantkoenig Dec 22, 2016

pyhedgehog Dec 22, 2016 •

edited

Loading

ethantkoenig Dec 23, 2016

ethantkoenig Dec 22, 2016

pyhedgehog Dec 22, 2016

ethantkoenig Dec 22, 2016

pyhedgehog Dec 22, 2016

ethantkoenig Dec 22, 2016

pyhedgehog Dec 24, 2016

coveralls commented Dec 22, 2016 •

edited

Loading

coveralls commented Dec 22, 2016 •

edited

Loading

ethantkoenig Dec 23, 2016

pyhedgehog Dec 24, 2016

ethantkoenig Dec 23, 2016

pyhedgehog Dec 23, 2016

ethantkoenig Dec 23, 2016

pyhedgehog Dec 24, 2016

coveralls commented Dec 23, 2016 •

edited

Loading

pyhedgehog commented Dec 24, 2016

ethantkoenig commented Dec 24, 2016

coveralls commented Dec 24, 2016 •

edited

Loading

pyhedgehog commented Dec 24, 2016

ethantkoenig commented Dec 24, 2016

coveralls commented Dec 24, 2016 •

edited

Loading

pyhedgehog commented Dec 24, 2016

Add API for token management #2

Add API for token management #2

Conversation

pyhedgehog commented Dec 22, 2016

coveralls commented Dec 22, 2016 • edited Loading

Choose a reason for hiding this comment

pyhedgehog Dec 22, 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

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

coveralls commented Dec 22, 2016 • edited Loading

coveralls commented Dec 22, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 23, 2016 • edited Loading

pyhedgehog commented Dec 24, 2016

ethantkoenig commented Dec 24, 2016

coveralls commented Dec 24, 2016 • edited Loading

pyhedgehog commented Dec 24, 2016

ethantkoenig commented Dec 24, 2016

coveralls commented Dec 24, 2016 • edited Loading

pyhedgehog commented Dec 24, 2016

coveralls commented Dec 22, 2016 •

edited

Loading

pyhedgehog Dec 22, 2016 •

edited

Loading

coveralls commented Dec 22, 2016 •

edited

Loading

coveralls commented Dec 22, 2016 •

edited

Loading

coveralls commented Dec 23, 2016 •

edited

Loading

coveralls commented Dec 24, 2016 •

edited

Loading

coveralls commented Dec 24, 2016 •

edited

Loading