Conversation
|
Neat, but the naming doesn't match the ssh keypair constructor we already have. The class for the full ssh keypair is Have you considered having a static method in Keypair to act as a constructor, so we can use it like |
|
@carlosmn does that look better? |
|
Yeah. Though you took both of my suggestions when I meant them as alternatives. Having simply What I was thinking a an alternative to that was having But having the |
|
@carlosmn Done. |
There was a problem hiding this comment.
One of these lies is redundant. If we're going to rely on the superclass' constructor, then there's no need for us to store the username explicitly.
|
Thanks for this patch! |
|
@carlosmn Anything outstanding to get this merged or do is it just a matter of you having the cycles to take a look (I know that feeling)? |
There was a problem hiding this comment.
I'd rather see this choose based on the values returned from .credential_tuple rather than based on inheritance. It's not too big a deal, but the idea behind these is that each instance tells us about what it's doing, rather than rely on the type.
There was a problem hiding this comment.
We thought about this and weren't sure which way to go. The only thing useful we could detect is tuple[1:] are all None. We went with this way as this is what the Ruby libgit2 bindings do.
Happy to change this to trigger off pubkey & privkey being None instead if you'd prefer.
There was a problem hiding this comment.
Yeah, that's how I'd go. This is designed so you can use your own classes which get the data from wherever you need (disc, the user, a database, whatever), and there's no need for them to hang off of these classes, all they need to do is have this attribute with the data we need.
There was a problem hiding this comment.
So something like this diff then?
@@ -455,9 +456,13 @@ def get_credentials(fn, url, username, allowed):
elif cred_type == C.GIT_CREDTYPE_SSH_KEY:
name, pubkey, privkey, passphrase = creds.credential_tuple
- err = C.git_cred_ssh_key_new(ccred, to_bytes(name), to_bytes(pubkey),
- to_bytes(privkey), to_bytes(passphrase))
-
+ if pubkey is None and privkey is None:
+ err = C.git_cred_ssh_key_from_agent(ccred, to_bytes(name))
+ else:
+ err = C.git_cred_ssh_key_new(ccred, to_bytes(name),
+ to_bytes(pubkey), to_bytes(privkey),
+ to_bytes(passphrase))
else:
raise TypeError("unsupported credential type")
There was a problem hiding this comment.
Yeah, that should work just fine.
|
Great, 👍 |
Introduce the ability for pygit2 to use the SSH agent for authentication.
Example use:
import pygit2
x = pygit2.credentials.SSHKeyFromAgent('git')
r = pygit2.clone_repository('ssh://git@github.com/some-org/some-repo', '/tmp/something', credentials=x)