#pyramid

offby1: well it's missing tests

offby1: that would be 1 beer

offby1: also what are you doing with the SignedCookieSessionsFactory and that random test stuff

offby1: you also have a hardcoded ip address

anything really; thanks.

let's take those one at a time

yes,it's missing tests; I've never learnt how to write tests for a web site ;-|

perhaps I should describe the point of all this: I have a little URL shortener (teensy.info) that, I've noticed, is attracting a lot of spam, so I'm hoping that reCAPTCHA will keep out the spam

offby1: pyramid docs have examples for the pyramid bits ... you can also use robotframework and selenium for the integration ones for the site

honestly, I'd rather do a casual five-minute fiddling-by-hand session than spin up selenium :-|

it's up to you

I find it annoying to do more then 3 times

plus you have to remember test cases

I too might, but I've only done it once or twice so far :)

offby1: http://docs.pylonsproject.org/projects/pyramid/...

so now the SignedCookieSessionsFactory is me trying to ensure that, if you've checked the recaptcha box once, that you won't have to again

seems to work :)

and the hard-coded IP address ... I don't know any way around that.

but are you seeding it randomly everytime

offby1: the ip should be in the .ini file at least

I'm seeding it randomly every time the server starts, which is not often.

ah, putting the IP address in the .ini file isn't a bad idea.

better yet: determining it at startup time would be nice (but that's surprisingly hard to do, in my experience)

offby1: so 1 problem with having the seed for signed cookie different is there is no guarantee your server process has not restarted

true, but ... so what?

Like I said, it only restarts rarely

people would lose the session

this isn't national defense

okie

again: so what

well you wanted feedback

it's not an argument

it's an opinion with some pros and cons

it's just a convenience. I don't even have any evidence that anyone besides me (and the spammers) ever enters anything into this

you get to decide

okie dokie

then it's your call

just laying out some implications :)

yep, thanks; I'm genuinely grateful you're lookin' at it

and I'm serious about the beer, if there's a convenient way (bitcoin? Amazon payments?)

also I would store the recapture secret in the config and load it in main

instead of global

you can always access it

from there

maybe add a property or method to the config

what's the benefit of that, though?

other than that it's more like how I'd do it if this were a real app?

it's passed in, easy to test

ah, true

also the way you have it now, it is not thread safe

and your server is threadful ;)

hm, now that's something

could you spell out what you mean by "not thread safe"?

presumably two simultaneous requests could interfere with each other... somehow

yes

they will

I don't see how though

am I mutating global state somewhere?

I am guessing _g_recaptcha_secret

well ... I only set that once, at startup; I'm assuming that the main function that sets it only gets invoked once

that should be fine

but personally I'd add it to the config

your configurator instance is there to keep config for the app

keep your information there

also it let's you have more then 1 app in the process if you wish

now that does sound cleaner

and use it as a module

and use it as in modular way

and as I said more testable

now you're earning your beer

since you can just have a dummy config with values

what response['success'] return

is ithat a boolean

offby1: in your template do you want display: none, or do you want visibility: hidden ... they are different

visibility keeps the space the element occupies

hold on

``response['success']'' is a boolean that Google gives me, after I've asked them if the captcha was filled out by a human

goodwill: about the template: I don't know what I want, really. I just know that I don't want the big ugly recaptcha thing visible if the person has already clicked it.

I barely know web programming :)

display:none might be better, particularly if it causes the thing to not take up any space

let's see

okie

I suggest display: none then

yeah, trying that now, and it just feels better

offby1: you have been in this channel for years, I never knew you were not doing web

well, of course I was; but I was mostly doing backend stuff

web services

I see

this URL shortener is a hobby project to force me to learn a smidgen about HTML and JS

:)

*nods*

offby1: in verify request what does line 51 return?

unfortunately I've changed jobs, and probably won't be using Pyramid at the current one

True/False?

hold on lemme check

offby1: you no longer at Amazon?

no, I just _started_ at Amazon; that's the place that probably doesn't use Pyramid

(I mean I probably could if I insisted, but ...)

my group uses some flask and some django

anyway.

line 51 indeed returns a bool

well if they got django and flask, time to add pyramid

cause clearly they are not mono-form

"if your function signature has ten arguments, you probably forgot one"

they're kinda loosey-goosy: teams can use whatever they want (except PHP -- seriously, it's banned)

lotta ruby, lotta java

okie, good on the verify_request, just wanted to know the return is consistent

I think that's most of my feedback

so to recap

(and take it as is)

1. cookie secret seed should be consistent across restarts

*nod

2. recapture secret should be in config not global

*nod nod (haven't yet figured out how to do that, but I assume it's config.settings)

3. whitelist ip should be in confifg not global

*nod

4. use display: none

already did

5. test all the things

awww

that was a _much_ more professional review than I was expecting

5. add docstrings with parameter and return docs to all the functions

I'm more gung-ho at work

since after all they pay me, and other people occasionally even read my code

but I might as well use my "best practices" in this project too

since it's Out There taking traffic (sometimes up to five requests per day! I need to upgrade the hardware!!)

also I would not use: here = os.path.abspath(os.path.dirname(__file__))

that recapture can also be in config

or at least the file path to it should be

there is nothing more then ops people hate then config settings inside the repos

hey, I thought I was doing well, not checking the secret into the repo

offby1: so typically settings like this are kept in secrets vault software (like KeyWhiz or Hashicorp Vault)

sure, but ... that's overkill for this

right