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

Feature: Enhanced plugin system #627

Open
cc-a opened this issue Oct 24, 2024 · 7 comments
Open

Feature: Enhanced plugin system #627

cc-a opened this issue Oct 24, 2024 · 7 comments
Labels
documentation documentation to be updated enhancement Improvement to existing feature

Comments

@cc-a
Copy link
Contributor

cc-a commented Oct 24, 2024

Description

Coldfront's current plugin mechanism is based on creating standalone Django apps and then requires the following modifications to the coldfront codebase:

  • editing coldfront/config/urls.py to configure urlpaths.
  • adding settings for the plugin to coldfront/config/plugins.
  • editing coldfront/config/settings.py to import the app settings.

The need to edit files within the coldfront codebase presents challenges for both development of plugins and for deployment of coldfront instances with third party plugins. For instance see ImperialCollegeLondon/coldfront_development_environment for an example of the additional infrastructure required to create a reproducible development environment.

Improving the above boils down to improving the handling of settings and url patterns for plugins.

Settings

A fundamental question here is why include settings from plugins at all? The current scheme promotes a pattern of plugins having access to the global settings which is not modular. It should be possible to set configuration for plugins from the global settings but I'd argue plugins in general shouldn't be changing global settings as this could lead to conflicts and unexpected behaviour.

A model to recommend to plugins for handling settings might be how django-debug-toolbar does things. It is a standalone app that has an internal settings module. Settings are accessed within the app via settings.get_config which loads a set of default app settings and overrides them based on values from the global settings.

URL Patterns

As there is a standard location (urls.py) for apps to define urlpatterns, their discovery and loading could be automated relatively easily via some importlib magic and (in the case of pip installed plugins) via entrypoints. Loading could be predicated on whether an app has been added to INSTALLED_APPS.

Component

Other

Additional information

No response

@cc-a cc-a added documentation documentation to be updated enhancement Improvement to existing feature labels Oct 24, 2024
@cc-a
Copy link
Contributor Author

cc-a commented Oct 24, 2024

Should add if you'd like to see a mockup for the urlpattern loading I'd be happy to knock together a PR.

@aebruno
Copy link
Member

aebruno commented Oct 25, 2024

@cc-a Thanks for the detailed write up. Related to this, we're currently evaluating using DJP as it seems to be well polished and checks a lot of boxes in terms of providing an enhanced plug system. Perhaps we can use this issue to discuss pros/cons and other options for improving the way ColdFront can be extended.

@cc-a
Copy link
Contributor Author

cc-a commented Oct 28, 2024

DJP looks interesting (I'm just annoyed it's not on the first page of google results for "django plugins" so I didn't come across it). Looks like it would neatly handle settings and urlpatterns whilst also potentially handling INSTALLED_APPS and MIDDLEWARE. A few thoughts on DJP:

  • In terms of design and features it looks very solid. Building off of pluggy seems like a good decision and the code is clear and well written.
  • its very very new and a single author project so some questions about it's long term viability.
  • I stand by my take from above that letting plugins fiddle with global settings is a problematic pattern. It would be possible to use DJP without using that functionality however. Do you guys have any specific use cases that would require plugins to modify global settings?
  • It doesn't look to be compatible with the way some things are done currently (the way plugins are structured, the way some plugins are shipped as part of the core application and the requirement to explicitly enable plugins). So you'd either need to introduce breaking changes and refactor internally or maintain the current plugin infrastruture alongside DJP. That said it doesn't look like updating existing shipped/community plugins would be very onerous.

@cc-a
Copy link
Contributor Author

cc-a commented Nov 5, 2024

Curious if you guys have any thoughts on this? Happy to implement mockups of one or more approaches that you think may be viable.

@aebruno
Copy link
Member

aebruno commented Nov 6, 2024

I stand by my take from above that letting plugins fiddle with global settings is a problematic pattern. It would be possible to use DJP without using that functionality however. Do you guys have any specific use cases that would require plugins to modify global settings?

I don't believe there is any specific use cases that would require plugins to modify global settings. Is the concern here that plugins could inadvertently modify some setting that they don't necessary own? DJP has this hook which passes in the current settings, are you suggesting this should not be allowed and instead only allow plugins to modify their own settings?

It doesn't look to be compatible with the way some things are done currently (the way plugins are structured, the way some plugins are shipped as part of the core application and the requirement to explicitly enable plugins). So you'd either need to introduce breaking changes and refactor internally or maintain the current plugin infrastruture alongside DJP. That said it doesn't look like updating existing shipped/community plugins would be very onerous.

Yes, agree it would be a major breaking change. The thinking would be to create a new branch (likely v2.x) that would include these features as part of a new major release of ColdFront, which should allow plugin authors time to migrate/refactor changes.

Happy to implement mockups of one or more approaches that you think may be viable.

That would be great. Think it would be worthwhile to see a mockup of how DJP could work in ColdFront and an a custom approach that borrows some of these ideas from the other plugin projects? Thoughts?

@cc-a
Copy link
Contributor Author

cc-a commented Nov 20, 2024

I've put together a couple of PR's (#636 and #639) that compare using DJP with a hand rolled approach. They mostly end up doing a lot of the same things and djp makes it easier and provides a nicer user interface to developers.

Overall the plugin system could be greatly simplified if explicit support for putting third party plugins in the plugins directory was dropped in favour of a pip based mechanism. Combined with refactoring the internal plugins the plugin related functionality could be parred down quite a lot.

If you're interested in pursuing either approach I'd be happy to write some follow on issues to get into the broader details of implementation.

@aebruno
Copy link
Member

aebruno commented Nov 20, 2024

@cc-a Thanks SO much for this. We'll try and get these reviewed asap and get you some feedback.

Overall the plugin system could be greatly simplified if explicit support for putting third party plugins in the plugins directory was dropped in favour of a pip based mechanism. Combined with refactoring the internal plugins the plugin related functionality could be parred down quite a lot.

Sounds good, I'm open to the pip based install method and makes a lot of sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation documentation to be updated enhancement Improvement to existing feature
Projects
None yet
Development

No branches or pull requests

2 participants