Decouple extensions from Flask app.

[jezdez]

What type of PR is this? (check all applicable)

• Refactor
• Feature
• Bug Fix

Description

This separates the extension registry from the Flask app and also introduces a separate registry for preriodic tasks.

Related Tickets & Documents

Fixes #3466.
Example change in redash-stmo: mozilla/redash-stmo#25

[arikfr]

See some comments, but there is a higher level thing I want to discuss --

Wouldn't it be better to have a single entrypoint (redash_extension?) which returns a dictionary. This dictionary can have multiple keys, each defines a different type of possible extension:

{
    'celery_periodic_tasks': [...],
    'flask_extensions': [...],
}

The registry loads this once, and then exposes an API to query it for the different extensions based on the type of functionality you need.

Wdyt?

[jezdez]

See some comments, but there is a higher level thing I want to discuss --

Wouldn't it be better to have a single entrypoint (redash_extension?) which returns a dictionary. This dictionary can have multiple keys, each defines a different type of possible extension:

{
    'celery_periodic_tasks': [...],
    'flask_extensions': [...],
}

Yeah, admittedly this PR is a proposal, thank you for looking it through. I forgot to use the PR draft feature 😬

I wondered about how to structure the extension metadata, too. I think it boils down to whether we want to use "entry point groups" ("redash.extensions", "redash.periodic_tasks", like, some, others) as a format to separate extension use cases or whether we want to create an own metadata schema for extensions to treat as an Redash-internal artifact.

Since it's one of the community standards that is being adopted by the Python stdlib via the importlib.metadata, it'll be around for a long time.

Anyway, for this round of the PR I leaned to the first format of using entry point groups since I was worried that we'd reinvent the wheel when we have to parse and handle the returning dicts that the single extension return.

While the entry point system doesn't specifically say anything about the return type of entry point callbacks, in my experience the callback are usually only catering to one specific use case, instead of being an entrypoint for another layer of configuration. E.g. Other than the examples above Pygments for example uses multiple entry points for various extension features. So far I have not seen versioned extensions based on entry points out there, so it seems this is not something people have used it for.

The registry loads this once, and then exposes an API to query it for the different extensions based on the type of functionality you need.

Wdyt?

I'm fairly open to doing it differently of course, but I'm interested in you looking into the examples out there and consider the downsides of maintaining part of an extension system on the long run. I know we don't have many (any?) extensions out there yet, so it's a low risk decision right now.

Lastly -- to look from an even higher perspective -- none of these solution solve the Python/NPM parity problem yet, meaning how should we deal with client extensions that also need backend code, and vice-versa. My gut feel says we can't easily combine them (since we'd have to either have an integration layer in Python to install additional NPM modules, and hook them into webpack, or one in the client code, which seems weird, too).

[arikfr]

Thank you for the references. Let's go the way you implemented it: having separate entry points for different types of extensions. But let's make sure that only the "Flask" extensions (not sure how to call them) depend on a Flask app instance.

[jezdez]

@arikfr Is this something you'd be interested in merging before v7? We'd lose the ability to add those periodic tasks if not..

[arikfr]

See comments.

I don't mind having this in v7 if it's finalized in time.
But can't you pick any commit to base your release from?

[arikfr]

Why do we need to load_bundles when we init the app?

[jezdez]

Mostly for consistency with the bundle script and so that we can inspect redash.extensions.bundles when needed.

[jezdez]

@arikfr This would be ready for another round of reviews 😬

[jezdez]

@arikfr Is there anything I can do to move this ahead?

[jezdez]

This is to make sure npm is available in this step and is consistent with the other steps where bundles are collected.

[arikfr]

Thanks!