search

RunestoneInteractive / BookServer

This new server separates book page serving and api requests from the instructor interface in RunestoneServer.
Other
6 stars 37 forks source link

Crasher on server side grading #105

Closed bnmnetp closed 2 years ago

bnmnetp commented 2 years ago

I'm getting a ton of crashes in the log due to this code:

    # Do server-side grading if needed, which restores the answer and feedback.
    if feedback := await is_server_feedback(request_data.div_id, request_data.course):
        rcd = runestone_component_dict[EVENT2TABLE[request_data.event]]
        # The grader should also be defined if there's feedback.
        assert rcd.grader
        # Use the grader to add server-side feedback to the returned dict.
        ret.update(await rcd.grader(row, feedback, user.is_exam_mode))

The error is 'AuthUserValidator' object has no attribute 'is_exam_mode'

In all of bookserver this is the only place that is_exam_mode is used. It is not in the model. Nor is it in RunestoneServer code.... Maybe I"m missing a PR? Does it even make sense that is_exam_mode would be an attribute of the user??

bjones1 commented 2 years ago

Oops, I didn't mean to commit that. It exists only on my server, for giving exams/high-stakes assessments. Just pass False where it appears. I'm away from my desk but will send a PR when I can.

On Sat, Jul 16, 2022, 11:57 AM Bradley Miller @.***> wrote:

I'm getting a ton of crashes in the log due to this code:

# Do server-side grading if needed, which restores the answer and feedback.
if feedback := await is_server_feedback(request_data.div_id, request_data.course):
    rcd = runestone_component_dict[EVENT2TABLE[request_data.event]]
    # The grader should also be defined if there's feedback.
    assert rcd.grader
    # Use the grader to add server-side feedback to the returned dict.
    ret.update(await rcd.grader(row, feedback, user.is_exam_mode))

The error is 'AuthUserValidator' object has no attribute 'is_exam_mode'

In all of bookserver this is the only place that is_exam_mode is used. It is not in the model. Nor is it in RunestoneServer code.... Maybe I"m missing a PR? Does it even make sense that is_exam_mode would be an attribute of the user??

— Reply to this email directly, view it on GitHub https://secure-web.cisco.com/1b5peHyPtn4EyglMQo2LbRiyu1ASSpYXeHvKBU7G2T_6W65iZ9QtWgDNMv2igbFTqVj1fbmmqmZeeh_0FVCB7of_BEDSKU9Vf13ZBBFxLsBMNLYzSzpxFbXpQVu1_3_04VRYAoRH8Dng20mSOXvhirH2kd_KtI2HRD4tkpvpoRmwo4WOiR-G61vUgetnmI2PSdJwIwepVSUgIDMvbwvxNt8JFGXfIoW9aHlB8GeK5NoWxjFl-xJCLuj7UV7A-wDgRU8knbVLaTO-Q_lEpRkquiKIFLFuUkKtl_TlyOiw-XlLnCo_sLW1J7TZmm_tZIzO6/https%3A%2F%2Fgithub.com%2FRunestoneInteractive%2FBookServer%2Fissues%2F105, or unsubscribe https://secure-web.cisco.com/1Vpxx6r73Px-7C_CWJh_Z5CNc1Wy5VHJW-3M4KZBqP82_5bPHTh36THSlCi04rdnYvG6RE3ZwdpaysxtQDrVdm0r_mbLDe6Rcfyp7OxhIuZnXiy1A--dJ_nqIDJDnkom4RbKYJRMzMIq3RD23rkqHWP-leI04TdtWbwKSBJxOTMtiFv8dwXXBzgBCyIDWf7_LpMexjwznSqo4sccom-VivYQ6FwrWnxaCnBETsst2AUgI2lyOw46PzyCPHanVlyVCVFC8eWCtWsaWYfiwN5SPRwWNeW-hYQE8wScuELXHD8Am8aYsQmz7QhH64hH66xc9/https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAAQG6EG5LAZ5VHJ5JWSNXZLVULSVRANCNFSM53YLCUTQ . You are receiving this because you were assigned.Message ID: @.***>

bnmnetp commented 2 years ago

OK, I did a quick release with it set to False.

bjones1 commented 2 years ago

Thanks!

bnmnetp commented 2 years ago

It turns out that just pushed the problem upstream... Now getting tons of fitb_feedback() takes 2 positional arguments but 3 were given

  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/middleware/errors.py", line 159, in __call__
    await self.app(scope, receive, _send)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/middleware/base.py", line 25, in __call__
    response = await self.dispatch_func(request, self.call_next)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/bookserver/main.py", line 103, in get_session_object
    response = await call_next(request)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/middleware/base.py", line 45, in call_next
    task.result()
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/middleware/base.py", line 38, in coro
    await self.app(scope, receive, send)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/middleware/base.py", line 25, in __call__
    response = await self.dispatch_func(request, self.call_next)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/fastapi_login/fastapi_login.py", line 440, in user_middleware
    return await call_next(request)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/middleware/base.py", line 45, in call_next
    task.result()
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/middleware/base.py", line 38, in coro
    await self.app(scope, receive, send)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/exceptions.py", line 82, in __call__
    raise exc from None
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/exceptions.py", line 71, in __call__
    await self.app(scope, receive, sender)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/routing.py", line 580, in __call__
    await route.handle(scope, receive, send)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/routing.py", line 241, in handle
    await self.app(scope, receive, send)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/starlette/routing.py", line 52, in app
    response = await func(request)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/fastapi/routing.py", line 226, in app
    raw_response = await run_endpoint_function(
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/fastapi/routing.py", line 159, in run_endpoint_function
    return await dependant.call(**values)
  File "/srv/web2py/applications/runestone/.venv/lib/python3.9/site-packages/bookserver/routers/assessment.py", line 108, in get_assessment_results
    ret.update(await rcd.grader(row, feedback, False))

We need to get this fixed the right way.

bnmnetp commented 2 years ago

I suspect that rather than passing false I should just eliminate that parameter.

bjones1 commented 2 years ago

Looking...

bjones1 commented 2 years ago

Yes, that's the correct fix.

bjones1 commented 2 years ago

(In 22781e75 that you pushed)

bnmnetp commented 2 years ago

I am super confused... I could not figure out why there were 170,000 crashes over two weeks for something involving server side grading... But It appears that somehow server side grading of fill in the blank questions is now the default??

The crashes cause things to behave normally and revert to client side grading, but I'm now worried what will happen.

Did you make the change to server side being the default intentionally??

bjones1 commented 2 years ago

Definitely not! The code that decided whether to do server-side grading is the is_server_feedback function in bookserver/crud.py. This code looks to see if the feedback field of the questions table for the question to grade has any content (plus a few other criteria). If so, it's graded server-side; otherwise, it's graded client-side. AFAIK, the only place contents was put in this field was by passing a fourth parameter to addHTMLToDB in RunestoneComponents/runestone/server/componentdb.py. I only see this in RunestoneComponents/runestone/lp/lp.py and RunestoneComponents/runestone/fitb/fitb.py. It looks like the code in fitb.py was changed at some point to always fill this in; I'm trying to dig out why/when this occurred now.

bjones1 commented 2 years ago

I think this is a bug I introduced long ago in fc11c2c2. The intent: only include feedback if the book is built for server-side grading. But this commit provides feedback no matter what. I'm not sure why this hasn't popped up sooner as a problem -- have we been grading server-side all along? Still looking.

bjones1 commented 2 years ago

I'll work on a fix for this when building the book (don't include feedback unless runestone_server_side_grading is True) tomorrow.

bnmnetp commented 2 years ago

Thanks.

So that appears to be a book-wide option. I would have thought this would be an option on the directive to make it server side. So that you could have server side grading on timed assessments in a book, but questions that are just simple check your understanding could be done on the client.

I'm guessing that we have been doing this all along, but that I didn't notice it until I started getting all of these crashers due to the is_exam_mode parameter. Then the question in the back of my mind all vacation was why so many? I didn't think very many authors used it, and I just verified that there is only one book in all of the books on academy that has the runestone_server_side_grading set to True.

bjones1 commented 2 years ago

I agree that having a per-problem option would be a good idea, and easy to implement -- simply omitting the feedback field marks a problem as client-side graded, while including feedback makes it server-side graded.

Then the question in the back of my mind all vacation was why so many? I didn't think very many authors used it, and I just verified that there is only one book in all of the books on academy that has the runestone_server_side_grading set to True.

The current code should check runestone_server_side_grading to decide if it should set feedback, but it doesn't. I think this explains what's going on. I'll create a PR to fix this.