#hypothes.is

dwhly: new iteration of site up: http://ec2-52-10-189-44.us-west-2.compute.amazo...

jakehart: awesome

jakehart: yt?

perhaps I'm being special, but I don't think notifications are working on master

ujvari: are you about?

nickstenn: yes

notifications are behaving really oddly, and I think I've finally worked out what's going on

Oh?

If I sign up to an application at "localhost", then I can log into the same application at "127.0.0.1"

but because the notifications package does crazy shit with acct URIs rather than just JOINing with the user table, the subscriptions aren't found

I'm not sure this is strictly a problem with the notifications package

but it's one place where the problem manifests itself

So,... it was a long term decision to decouple subscriptions and user table

but yes, I hate this @domain problem

Cumbersome and terrible all the way down

if we *had* managed to decouple accounts from notifications then I might be in favour

but we haven't

hmm, yes, the gateway, sadly yes

they're just fundamentally not decouplable -- the notification package needs to know a user's email address

yes. Just one question. And it doesn't cause problem with the annotations?

so you wouldn't get: acct:user@127.0.0.1 and

yes, it does

acct:user@localhost ?

ah okay

I was always telling

to get a domain from the backend

and use always that

so what are you planning to do? How can I help?

i'm not really sure at the moment

I'm not even sure I fully understand what's going on

Okay, I can help on that. Which part you don't understand?

s/on/with

I'm running FEATURE_NOTIFICATION=True ./run

And I create two user accounts

with user A, I create an annotation

with user B I reply to user A's annotation

I expect to see an email printed to the console

but I don't

are we still logging the mail sent?

the AnnotationEvent subscriber in h.notification.reply_template (?) doesn't even seem to get invoked

user A has a checked checkbox in their notification settings

(as does user B, which I suppose isn't relevant)

give me a few minutes, I want to try it out

Wow... I think we no longer have the AnnotationEvent?

Is it possible

?

no

h/events.py

and it's triggered in h/api.py

just grep for it

hmm

okay

checking

something jolly fishy is going on, though

yes, I suspect we do not trigger the event

(still checking)

Okay, it gets triggered

in the api

Now that is strange, the streamer.py gets the event but the reply_template.py doesn't

yep

hmm

seems to have broken in 29c1500

hmm

so an import conf issue.. hmm

I'll quote your favorite sentence: We need at least one test case to check if notifications are still working

but this appears at the moment to be something fucked with pyramid

i mean, maybe it's not

but I can't see what the problem is

Yes, me neither at this moment

ohhhhhh

ohhhh, shit

?

i bet you I know what the problem is

request !== request

api is a separate wsgi app

I beg your pardon?

...ooooh

with a separate registry

...

oooh..

oh man

that's...

oooh joooy

that's going to be a nightmare to fix

Agreed

hang on, though

why does the streamer work?

hmm.. good question

nickstenning: stupid question

in your development.ini

have you set the feature to true?

it is set to false by default

[13:34:13] <+nickstenn>I'm running FEATURE_NOTIFICATION=True ./run

yes, then next

if you check

create_api() in the app.py

then the notification feature is not added to there

ohhhh

i think you might be right

in fact I think you must be right

good work

the issue is exactly what we said it was

and it's that the subscribers aren't registered in the API application

jesus

what a nightmare

oh yes

oh ffs, I don't even understand what the point of separating the fucking API is

in order to make this work we need to include the notifications subscriber

which needs access to the database

which means we need to include the accounts package

I just don't get why we're pretending these are all beautifully decoupled when they're not

Hmm, I think this something we left behind, originally there were more refacts to be expected to make the separation clean (Now I remember) but they never came, sadly.

s/refactorings/changes/, but whatever

it's just broken

ugh

What should be done? Notifs are broken on prod with this

I'm working on it

Ok, do you know a fast way for decoupling?

hypothesis/h#4969 (fix-og-title - d576c14 : BigBlueHat): The build was fixed.

Change view : https://github.com/hypothesis/h/compare/ac43791...

Build details : http://travis-ci.org/hypothesis/h/builds/52281732