Provide reviewboard-extensions interface.

Bug #1359931 reported by Eric Snow
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
reviewboard (Juju Charms Collection)
Triaged
Low
Unassigned

Bug Description

The key one for me would be a "reviewboard-extensions" interface. Reviewboard has versatile extension support. Writing a charm that makes use of that requires a little bit of interaction with the reviewboard charm. This new interface would facilitate that. I imagine it would be something like this:

provides:
  reviewboard-extensions:
    interface: reviewboard-extensions
    gets: [settings] # a mapping of names to values.

The charm would add two hooks:
 * reviewboard-extensions-relation-changed - update reviewboard's settings.py with "settings";
 * reviewboard-extensions-relation-joined - force reviewboard recognize the extension (reload, e.g. touch settings.py).

"settings" could conceivably go in a "reviewboard" interface or even a generic "settings" interface that would perform that "-relation-changed" role:

provides:
  reviewboard: # or settings
    interface: reviewboard # or settings
    gets: [settings]

A plain "reviewboard" interface would probably be a good idea regardless. It would at least allow consumers to know when reviewboard was ready to go.

Revision history for this message
Eric Snow (ericsnowcurrently) wrote :
Download full text (3.7 KiB)

I imagine that the hook code would be something like this:

INJECTED_WARNING = """
#------------------------------------------------------------------------------
# The following is the import code for the settings directory injected by Juju
#------------------------------------------------------------------------------
"""

PKG = '_juju_extensions_settings'
PKG_DIR = os.path.join(os.path.dirname(path_to_rb_settings_py), PKG)

def open_settings(extname):
    filename = os.path.join(PKG_DIR, extname + '_settings.py')
    try:
        return open(filename, 'r+')
    except IOError:
        if os.path.exists(filename):
            raise

    if not os.path.exists(PKG_DIR):
        os.mkdir(PKG_DIR)
        with open(path_to_rb_settings_py, 'a') as settings_py:
            settings_py.write(INJECTED_WARNING)
            settings_py.write("from {} import *".format(PKG))
    extfile = open(filename, 'w+')

    with open(os.path.join(PKG_DIR, '__init__.py'), 'a') as initfile:
        initfile.write('from .{} import *\n'.format(extname))

    return extfile

def set_settings(extname, settings):
    # inspired by what the django charm does.

    lines = []
    for name, value in settings.items():
        line = '{} = {!r}\n'.format(name, value)
        try:
            ast.literal_eval(line)
        except SyntaxError:
            hookenv.log('bad setting: {!r}'.format(line))
            return # fail?
        lines.append(line)

    with open_settings(extname) as extfile:
        extfile.writelines(lines)
        extfile.truncate()

    # Force RB to reload settings.py.i
    touch(path_to_rb_settings_py)

def clear_settings(extname=None):
    if extname is not None:
        filename = os.path.join(PKG_DIR, extname + '_settings.py')
        if os.path.exists(filename):
            os.remove(filename)
            extline = 'from .{} import *\n'.format(extname)
            with open(os.path.join(PKG_DIR, '__init__.py'), 'r+') as initfile:
                last = 0
                for line in initfile:
                    if line == extline:
                        break
                    last += len(line)
                lines = []
                for line in initfile:
                    lines.append(line)
                initfile.seek(last)
                initfile.writelines(lines)
                initfile.truncate()
    else:
        if os.path.exists(PKG_DIR):
            os.rmdir(PKG_DIR)
            # remove import from path_to_rb_settings_py?

def related_name():
    """Return the name of the extension in the current relation."""
    # We just use the service name.
    unit = hookenv.related_units()[0]
    service = unit.partition('/')[0]
    return service

def _handle_relation_settings():
    requested = hookenv.relation_get('settings')
    if not requested:
        # no settings to add
        return

    name = related_name()
    settings = hookenv.relation_get('_all_settings') or {}
    settings.update(requested)
    try:
        set_settings(name, settings)
    except Exception as e:
        hookenv.log("could not update reviewboard settings: {}".format(e))

# hooks

@hooks.hook('reviewboard-extensions-relation-joined')
def re...

Read more...

Revision history for this message
Adam Collard (adam-collard) wrote :

I'd like to keep the scope of this bug to just the extensions interface. A generic "make reviewboard specific interfaces" is too broad..

summary: - Provide reviewboard-specific interfaces.
+ Provide reviewboard-extensions interface.
Changed in reviewboard (Juju Charms Collection):
status: New → Triaged
importance: Undecided → Low
Revision history for this message
Eric Snow (ericsnowcurrently) wrote :

That's fine with me, Adam. Thanks for taking a look. For now I'm going to hack reviewboard's settings.py in the charm I'm writing for an extension, but it would be nice if that wasn't necessary. :)

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.