Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The "all registrations" reports are returning Nginx 404s #115

Open
adavidramos opened this issue Dec 31, 2013 · 11 comments
Open

The "all registrations" reports are returning Nginx 404s #115

adavidramos opened this issue Dec 31, 2013 · 11 comments
Labels

Comments

@adavidramos
Copy link
Member

We have some pages that return a list of every registration in a session, with information about the registrants. These are slow to render, perhaps because they're database-expensive or because we're doing something dumb.

Anyway, the URLs for these return 404s from Nginx. I think that Django is busily trying to produce the data but Nginx gets tired of waiting and sends the user a 404.

See:

http://knowledgecommonsdc.org/classes/staff/registrations/session/1209/

@nyetsche
Copy link
Contributor

nyetsche commented Jan 3, 2014

I just checked the nginx configuration, helpfully stored here - https://github.com/knowledgecommonsdc/kcdc3/blob/master/misc/nginx-sites-enabled-kcdc.conf - and I don't see any timeouts explicitly set, which tells me that it should be 60 seconds. http://nginx.org/en/docs/http/ngx_http_proxy_module.html#proxy_connect_timeout

That being said, I:

a) measured it with a wall clock and it times out in 30 seconds
b) believe that even if we increase the timeout, it's still "wrong".

Which class/link does this refer to? I need to send you my new SSH keys, right now I can't log into the server.

@nyetsche
Copy link
Contributor

nyetsche commented Jan 3, 2014

Is it possible that class (or "slug" in the Django parlance) doesn't actually exist? I found this curious artifact in apps/classes/views.py:

# display a list of registrations for a given session
class SessionAdminListView(ListView):

    template_name = "classes/staff_session_list.html"
    context_object_name = "session_list"
    model = Session

    def get_context_data(self, **kwargs):

        context = super(SessionAdminListView, self).get_context_data(**kwargs)
        context['session_list'] = Session.objects.all()

        # is the user staff?
        if self.request.user.is_staff:
            return context
        else:
            # TODO this should really return a 403
            return HttpResponse()

    @method_decorator(login_required)
    def dispatch(self, *args, **kwargs):
        return super(SessionAdminListView, self).dispatch(*args, **kwargs)

It's that "should return a 403" comment that leaves me puzzled. But the melatonin is kicking in so I can't diagnose further tonight.

@adavidramos
Copy link
Member Author

It's possible that the slug doesn't exist.

The "should return a 403" - right now, non-admin users see a blank page. We should instead tell them that they are trespassing.

This issue might not be relevant anymore - see issue #119.

@nyetsche
Copy link
Contributor

nyetsche commented Jan 7, 2014

Poking at this since I'm bored and cold. So very cold.
First, I misidentified the view; based on urls.py it is:

    url(r'^staff/registrations/session/(?P<slug>[A-Za-z0-9_-]+)/$', RegistrationListView.as_view()),
class RegistrationListView(ListView):

    template_name = "classes/staff_registration_list.html"
    context_object_name = "registration_list"
    model = Registration

    def get_context_data(self, **kwargs):

        context = super(RegistrationListView, self).get_context_data(**kwargs)
        context['events'] = Registration.objects.filter(event__session__slug=self.kwargs['slug'])

        # is the user staff?
        if self.request.user.is_staff:
            return context
        else:
            # TODO this should really return a 403
            return HttpResponse()

The SQL statement ultimately being run is

SELECT classes_registration.id, classes_registration.student_id, classes_registration.event_id, classes_registration.date_registered, classes_registration.waitlist, classes_registration.attended, classes_registration.cancelled, classes_registration.date_cancelled, classes_registration.date_promoted, classes_registration.late_promotion FROM classes_registration ORDER BY classes_registration.date_registered ASC;

Which, if I run it manually, returns 8033 rows. In 0.02 seconds. So...not too bad, but clearly more than django can handle, and more than we want.

I'm guessing we want a better filter?

@nyetsche
Copy link
Contributor

nyetsche commented Jan 7, 2014

I'm thinking now that:

        context['events'] = Registration.objects.filter(event__session__slug=self.kwargs['slug'])

is what's wrong. Specifically event__session__slug=self.kwargs['slug']) - doesn't that ultimately end up as WHERE event.session__slug clause? I don't have a test environment so I'm masochistically computing this in my head (and watching similar runs in an empty sqlite3). The classes_registrations table not have an event, session, or slug field. The classes_session table does has a slug field.

@nyetsche
Copy link
Contributor

nyetsche commented Jan 7, 2014

I take it back, after going through all of models.py, I do see that there's correctly a Event class, containing a foreign key for Session, containing a foreign key Slug. So maybe it's "all good" but really just just the large query that 404s.

@nyetsche
Copy link
Contributor

nyetsche commented Jan 7, 2014

Well, 'slug' in the context of Registration object is going to be different than a slug in the Session object than in the Event object. The Session object does has a numerical slug, aka 1409, that maps to what this view is called with. But the slug in Event is a 50-char name.

Maybe that should be filter(event__session_id__slug=self.kwargs['slug'])?

@adavidramos
Copy link
Member Author

No dice for filter(event__session_id__slug=self.kwargs['slug']).

There is a FilteredTeacherAdminListView() method that does filter correctly. I've tried to borrow the same line in RegistrationListView():

event__session_id__slug
event__session_id__slug__iexact
event__session__slug
event__session__slug__iexact

but to no avail.

__iexact just asks for a case-insensitive match, so I don't quite know why we'd need to be using it here anyway.

@nyetsche
Copy link
Contributor

Finally got a VM/vagrant instance running. It does eventually return, it just takes a /really/ long time and I guess our low tier instance is slower than my Mac.

@nyetsche
Copy link
Contributor

Are you sure event__session__slug didn't work? That was my eventual solution, put up here: #123

I also spent time going through Django docs; an alternate 'solution' I came up with is to add a session foreign key to the Registration model and just work from that. Of course, then we have to manually populate it.

The pull request works on my VM, but there's something "not quite right" with the state of the code - it's pretty difficult to go from a clone to a test environment, even after I manually copied over all the static files.

@adavidramos
Copy link
Member Author

Pretty sure event__session__slug didn't work - but I think you've also
changed the context object, which might have been the key.

Will look at this more.

Thanks,
Dave

Matt Lesko wrote:

Are you sure |event__session__slug| didn't work? That was my eventual
solution, put up here: #123
#123

I also spent time going through Django docs; an alternate 'solution' I
came up with is to add a session foreign key to the Registration model
and just work from that. Of course, then we have to manually populate it.

The pull request works on my VM, but there's something "not quite right"
with the state of the code - it's pretty difficult to go from a clone to
a test environment, even after I manually copied over all the static files.


Reply to this email directly or view it on GitHub
#115 (comment).

@adavidramos adavidramos added this to the Internal Tools milestone Mar 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants