fedora-infra / fas

Fedora Account System
https://admin.fedoraproject.org/accounts
GNU General Public License v2.0
40 stars 50 forks source link

Issue#96 #131

Closed DhritiShikhar closed 9 years ago

DhritiShikhar commented 9 years ago

Creates a requirements.txt

laxathom commented 9 years ago

Sound good.Will merge it manually to only take the latest.

laxathom commented 9 years ago

@ralphbean you mean at line 238?

ralphbean commented 9 years ago

@ralphbean you mean at line 238?

Yes :)

DhritiShikhar commented 9 years ago

@ralphbean could you please take a look at the patch?

laxathom commented 9 years ago

sound good for real now :) :+1:

ralphbean commented 9 years ago

This still looks incorrect to me. Once again, the value of self.valid_ssh_key is not being interpolated into the regex pattern.

This would only match ssh keys that have the literal string "valid_ssh_key" in them, which is not what we want. We want to match keys that have the literal strings "rsa" or "ssh-rsa" or "ecdsa", etc.. So, those strings are in a variable named valid_ssh_key which needs to be interpolated into the regex pattern before it is applied.

laxathom commented 9 years ago

hm, that's actually what the the .replace() does by adding the "|" which stand for OR. I did try it myself from a python console and it does works as expected.

pypingou commented 9 years ago

@laxathom it's the re.match() line that's problematic

laxathom commented 9 years ago

Ah, stupid me! Saw the passed string instead of the variable.

laxathom commented 9 years ago

hm, do not copy/paste my line. some caracters got stripped by markdown

DhritiShikhar commented 9 years ago

@laxathom could you please check if the patch is alright?

pypingou commented 9 years ago

@laxathom ping ?

laxathom commented 9 years ago

Sorry for the delay. Just did a manual merge by squashing your commits.