on mixins

We use mixins quite a lot in mozharness.

Mixins are a powerful pattern that allow you to extend your objects, reusing your code (more here). Think about mixin as “plugins”, you can create your custom class and import features just inheriting from a Mixin class for example:

class B2GBuild(LocalesMixin, PurgeMixin, B2GBuildBaseScript,
               GaiaLocalesMixin, SigningMixin, MapperMixin, BalrogMixin):

B2GBuild manages FirefoxOS builds and it knows how to:
* manage locales (LocalesMixin)
* how to deal with repositories (PurgeMixin)
* sign the code (SigningMixin)
* and more…

this is just from the class definition! At this point a we haven’t added any single method or property, but we already know how to do a lot of tasks and it’s almost for free!

So should we use mixins everywhere? Short answer: No.
Long answer Mixins are powerful but also they can lead to some unexpected behavior.

• first of all, using mixins is easy to fall in the “god object” trap
• They could make testing hard as shown in this example:

  class MixinA(object):
      def method_1(self):
          print "method MixinA1"
  
  class MixinB(object):
      def method_1(self):
          print "method MixinB1"
  
  class C(MixinA, MixinB):
      def __init__(self):
          print "I'm in C"
  
  class D(MixinB, MixinA):
      def __init__(self):
          print "I'm in D"
  
  if __name__ == '__main__':
      c = C()
      c.method_1()
  
      d = D()
      d.method_1()
  
  I'm in C
  method MixinA1
  I'm in D
  method MixinB1

object C and D have exactly the same parents and the same methods but their behavior is different, it depends on how the parents are declared.

This is a side effect of the way python implements the inheritance. Having an object inheriting from too many Mixins can lead to unexpected failures (MRO – method resolution objects) when the object is instantiated, or even worse, at runtime when a method is doing something that is not expected.
When the inheritance becomes obscure, it’s also becomes difficult to write appropriate tests.

How can we write a mozharness module without using mixins? Let’s try to write a generic module that provides some disk informations for example we could create the mozharness.base.diskutils module that provides useful information about the disk size. Our first approach would be writing something as:

class DiskInfoMixin():
    def get_size(self, path):
        self.info('calculating disk size')
        <code here>

    def other_methods(self):
        <code here>

and then use it in the final class

from mozharness.base.diskutils import DiskInfoMixin
...

class BuildRepackages(ScriptMixin, LogMixin, ..., DiskInfoMixin):
...
    disk_info = self.get_size(path)

Easy! But why are we using a mixin here? Because we need to log some operations and to do so, we need to interact with the LogMixin. This mixin provides everything we need to log messages with mozharness, it provides an abstraction layer to make logging consistent among all the mozharness script and it’s very easy to use, just import the LogMixin and start logging!
The same code without the using the LogMixin, would more or less be:

import logging

get_size(path):
    logging.info('calculating disk size')
    ...
    return disk_size

Just a function. Even easier.

… and the final script becomes:

from mozharness.base.diskutils import get_size
class BuildRepackages(ScriptMixin, LogMixin, ...,):
...
     disk_info = get_size(path)

One less mixin!
There’s a problem though. Messages logged by get_size() will be inconsistent with the rest of the logging. How can we use the mozharness logging style in other modules?
The LogMixin, it’s a complex class and it has many methods, but at the end of the day it’s a wrapper around the logging module, so behind the scenes, it must call the logger module. What if we can just ask our logger to use the python log facilities, already configured by mozharness?
getLogger() method is what we need here!

import logger
mhlog = logger.getLogger('Multi')
get_size(path):
    mhlog.info('calculating disk size')
    ...
    return disk_size

Mozharness by default uses this ‘Multi‘ logger for its messages, so we have just hooked up our logger into mozharness one. Now every logger call will follow the mozharness style!
We are half way trough the logging issues for our brand new module: what if we want to log to an arbitrary log level, for example, a quite common pattern in mozharness, is let the caller of a function, decide at what level we want to log, so let’s add a log_level parameter…

import logger
mhlog = logger.getLogger('Multi')
get_size(path, log_level=logger.INFO):
    mhlog.log(lvl=log_level, msg='calculating disk size')
    ...
    return disk_size

This will work fine for a generic module but we want to use this module in mozharness so there’s only one more thing to change: mozharness log levels are strings type, logging module levels are integers, we need a function to convert between the two formats.
For convenience, in mozharness.base.log we will explicitly expose the mozharness log levels and add function that converts mozharness log levels to standard log levels.

LOG_LEVELS = {
    DEBUG: logging.DEBUG,
    INFO: logging.INFO,
    WARNING: logging.WARNING,
    ERROR: logging.ERROR,
    CRITICAL: logging.CRITICAL,
    FATAL: FATAL_LEVEL
}

def numeric_log_level(level):
    """Converts a mozharness log level (string) to the corresponding logger
       level (number). This function makes possible to set the log level
       in functions that do not inherit from LogMixin
    """
    return LOG_LEVELS[level]

our final module becomes:

import logging
from mozharness.base.log import INFO, numeric_log_level
# use mozharness log
mhlog = logging.getLogger('Multi')

def get_size(path, unit, log_level=INFO):
    ...
    lvl = numeric_log_level(log_level)
    mhlog.log(lvl=lvl, msg="calculating disk size")

This is just an example on how to use the standard python logging modules.
A real diskutils module is about to land in mozharness (bug 1130336), and shouldn’t be too difficult, following the same pattern to create new modules with no dependencies on LogMixin.

This is a first step in the direction of removing some mixins from the mozharness code (see bug 1101183).
Mixin are not the absolute evil but they must be used carefully. From now on, if I have to write or modify anything in a mozarness module I will try to enforce the following rules:

• prefer composition over inheritance (see bug 1063532)
• do not inherit from more than n classes, if an object requires several parents, probably we could create some intermediate objects or the code needs a refactoring
• if it’s difficult to test, probably the code, can be structured in another way