yahoo-eng-team team mailing list archive

[Radomir Dopieralski <1278825@xxxxxxxxxxxxxxxxxx>]

Public bug reported:

There is an antipattern in Python, where to define a "virtual" method
people would raise a NotImplementedError exception in the body of the
method. This makes it impossible to use super() properly with that
method and, because of that, to use multiple inheritance, such as
mixins.

Horizon has several places where this is used, including some methods,
such as get_context_data, that are very likely to be touched by various
mixins. Those methods should return some default, empty value, instead
of raising an exception.

A typical implementation of get_context_data() in Django should look
something like this:

    def get_context_data(self, **kwargs):
        context = super(OverviewTab, self).get_context_data(**kwargs)
        context.update({
            'overcloud': 1,
            'roles': [1, 2, 3],
        })
        return context

This will break horribly if the superclass' get_context_data raises
NotImplementedError.

** Affects: horizon
     Importance: Undecided
     Assignee: Radomir Dopieralski (thesheep)
         Status: New

** Changed in: horizon
     Assignee: (unassigned) => Radomir Dopieralski (thesheep)

** Description changed:

  There is an antipattern in Python, where to define a "virtual" method
  people would raise a NotImplementedError exception in the body of the
  method. This makes it impossible to use super() properly with that
  method and, because of that, to use multiple inheritance, such as
  mixins.
  
  Horizon has several places where this is used, including some methods,
  such as get_context_data, that are very likely to be touched by various
  mixins. Those methods should return some default, empty value, instead
  of raising an exception.
  
  A typical implementation of get_context_data() in Django should look
  something like this:
  
-     def get_context_data(self, request, **kwargs):
-         context = super(OverviewTab, self).get_context_data(request, **kwargs)
-         context.update({
-             'overcloud': 1,
-             'roles': [1, 2, 3],
-         })
-         return context
+     def get_context_data(self, **kwargs):
+         context = super(OverviewTab, self).get_context_data(**kwargs)
+         context.update({
+             'overcloud': 1,
+             'roles': [1, 2, 3],
+         })
+         return context
  
  This will break horribly if the superclass' get_context_data raises
  NotImplementedError.

-- 
You received this bug notification because you are a member of Yahoo!
Engineering Team, which is subscribed to OpenStack Dashboard (Horizon).
https://bugs.launchpad.net/bugs/1278825

Title:
  Raising NotImplementedError makes it impossible to use super()
  properly

Status in OpenStack Dashboard (Horizon):
  New

Bug description:
  There is an antipattern in Python, where to define a "virtual" method
  people would raise a NotImplementedError exception in the body of the
  method. This makes it impossible to use super() properly with that
  method and, because of that, to use multiple inheritance, such as
  mixins.

  Horizon has several places where this is used, including some methods,
  such as get_context_data, that are very likely to be touched by
  various mixins. Those methods should return some default, empty value,
  instead of raising an exception.

  A typical implementation of get_context_data() in Django should look
  something like this:

      def get_context_data(self, **kwargs):
          context = super(OverviewTab, self).get_context_data(**kwargs)
          context.update({
              'overcloud': 1,
              'roles': [1, 2, 3],
          })
          return context

  This will break horribly if the superclass' get_context_data raises
  NotImplementedError.

To manage notifications about this bug go to:
https://bugs.launchpad.net/horizon/+bug/1278825/+subscriptions