It won't raise expected error when nestedly use course_view decorator

[dzhuang]

relate/course/utils.py

Line 683 in 63a25b6

raise RuntimeError(

If I decorate

relate/course/utils.py

Line 845 in 63a25b6

def render_course_page(pctx, template_name, args,

with course_view, it just failed with 404, not the expected error.

The _is_in_context_manager are unknow to each other in different CoursePageContext instances..

[dzhuang]

Can we just add an attribute _is_in_course_view to the HttpRequest object (i.e., pctx.request) instead, and raise the error at __init__ stage? The above tests failed at that stage and never has chance to __enter__.

[inducer]

I don't understand what the problem is. Could you explain?

[dzhuang]

In short, I can't find a situation that could trigger the exception.

[inducer]

If you do

with ...:
   with ...:
      pass

that should raise, no?

[dzhuang]

I tested with:

@coures_view
def func_a(pctx):
     ...
    return fuc_b(pctx)


@course_view
def func_b(pctx):
    ...

It doesn't raise the expected RuntimeError(), a 404 instead.

[inducer]

But each decorator creates a new CoursePageContext instance. I'm not surprised that doesn't raise.

[dzhuang]

It's also the case for context manager, unless we are with the same instance?

[inducer]

It can be, but it doesn't have to be. This form will make new instances (and hence won't conflict):

with CoursePageContext(...) as cpc:
   with CoursePageContext(...) as cpc2:
      pass

but this will:

cpc = CoursePageContext(...)
with cpc:
   with cpc:
      pass

It's perhaps an unlikely scenario to defend against, but I just don't like code that has the potential to be silently incorrect. (and it's easy to get into that situation if you're modifying state)

[dzhuang]

Got it. Thanks for the explanation.

[dzhuang]

Todo: add tests for this part.

[dzhuang]

Tested by #535 , closing.