Navigation Menu

Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin-ability for views #91

Merged
merged 14 commits into from Apr 12, 2021
Merged

Plugin-ability for views #91

merged 14 commits into from Apr 12, 2021

Conversation

create-issue-branch[bot]
Copy link
Contributor

@create-issue-branch create-issue-branch bot commented Apr 7, 2021

Add the possibility for views to be plugged-in to FlexMeasures by means of Blueprints.
Also adds the config option FLEXMEASURES_LISTED_VIEWS, so we can control what is shown in the menu. We cannot yet control which icon the menu entry will have (just use "info" for now).

This PR also fixes a few things in documentation and UI, which are too small for their own PR.

closes #90 and maybe #92

@nhoening
Copy link
Contributor

nhoening commented Apr 8, 2021

Here is a showcase file I made in <some_folder>/our_client/fmplugin/__init__.py, where the plugin is a Flask Blueprint.
All else that is needed for this showcase (not shown) is <some_folder>/our_client/fmplugin/templates/metrics.html which works just as other FlexMeasures templates.
I also showcased how to add a CLI function, to validate this approach. That also works.

from flask import Blueprint, render_template, abort

from flask_security import login_required
from flexmeasures.ui.utils.view_utils import render_flexmeasures_template


our_client_bp = Blueprint('our_client', 'our_client',
                      template_folder='templates')


# Showcase: Adding a view

@our_client_bp.route('/metrics')
@login_required
def metrics():
    msg = "I am part of FM !"
    return render_flexmeasures_template(
        "metrics.html",
        message=msg,
    )


# Showcase: Adding a CLI command

import click
from flask import current_app
from flask.cli import with_appcontext


our_client_bp.cli.help = "Our client commands"

@our_client_bp.cli.command("test")
@with_appcontext
def oc_test():
    print(f"I am a CLI command, part of FM: {current_app}")

@nhoening nhoening marked this pull request as ready for review April 8, 2021 15:06
@nhoening nhoening requested a review from Flix6x April 8, 2021 15:07
@nhoening
Copy link
Contributor

Note: I believe we might do without the fmplugin folder (so the linked folder is the plugin with an __init__.py). That would be preferable, right? Or are there scenarios where you want to store other things in that folder which are not the plugin and should not be confused with it?

@Flix6x
Copy link
Contributor

Flix6x commented Apr 11, 2021

Without fmplugin would be cleaner. If the user has a plugin module also containing files that shouldn't be registered, couldn't they just point to the FM specific file within their plugin?

Copy link
Contributor

@Flix6x Flix6x left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only main concern is that it appears as if all plugin views are public.

documentation/changelog.rst Show resolved Hide resolved
documentation/configuration.rst Outdated Show resolved Hide resolved
documentation/dev/plugins.rst Outdated Show resolved Hide resolved
documentation/dev/plugins.rst Outdated Show resolved Hide resolved
- Your plugin folder contains an 'fmplugin' folder with an __init__.py file.
- In this init, you define a Blueprint object called <plugin folder>_bp

TODO: Support multiple plugins.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it suffice to make the config setting a list of paths and loop through that list?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably. But I also would need to test that which is somewhat more effort. Actually, I believe it might take a while until someone needs it, or do you disagree?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hopefully not a lot of extra effort: I suggest the test would be to split up your one module defining both a UI view and CLI function into two modules. We'd indeed most likely be the first users of this plugin architecture, but I would prefer us to work in a modular fashion on small and dedicated plugins from the start, if possible. But fair enough, if your preference is to have multiple plugins be outside of the scope of this PR, let's open a separate ticket.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my eyes, one plugin and one private repo is what one would usually make per customer, and that's what we'll be doing for a few months. Of course, if our customers want custom stuff per client of theirs, then we should have more than one plugin per server. But that would also require our account data model (multiple accounts per server).

flexmeasures/ui/templates/defaults.jinja Show resolved Hide resolved
flexmeasures/ui/templates/defaults.jinja Outdated Show resolved Hide resolved
{% endfor %}


{% set show_queues = True if current_user.is_authenticated and (current_user.has_role('admin') or FLEXMEASURES_MODE == "demo") else False %}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Candidate for an explicit config setting, rather than checking for server mode.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the design problem here is that some views are only for authenticated users (that's the default) while others also require a specific role (right now that only touches the admin role).

/tasks is the only current example of the latter but there might be others. I'll think about this some more.

@nhoening
Copy link
Contributor

Yes, I believe it could be either a folder (needing an __init__.py to be imported) or a file. I'd recommend testing both out to be sure.

What I don't know is how paths are resolved from a file (e.g. to templates, but also other Python files or configs like .env). Often, you have other things like this, so my gut feeling right now is to stay with the folder approach and not support something else, as well.

@nhoening nhoening merged commit 134fb69 into main Apr 12, 2021
@nhoening nhoening deleted the issue-90-Plugin-ability_for_views branch April 12, 2021 11:25
@nhoening nhoening added this to the 0.4.0 milestone Apr 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin-ability for views
2 participants