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

Look for Unicorn components in all INSTALLED_APPS by default #210

Closed
nerdoc opened this issue Jun 9, 2021 · 6 comments
Closed

Look for Unicorn components in all INSTALLED_APPS by default #210

nerdoc opened this issue Jun 9, 2021 · 6 comments
Assignees

Comments

@nerdoc
Copy link
Contributor

nerdoc commented Jun 9, 2021

I'm just new to unicorn, and faced the first problem I see as newcomer. First there was only the "unicorn" app where components could live. Then, thanks to #15 there is a setting where you can define apps with components to look for.
But in a large scale application this is bad(TM).

Why can't unicorn just look at all installed apps and see if there is a components folder, hence load components from there? And any app could provide templates as well.

Is there any technical problem that prevents such a feature?

Background: I wrote a Django plugin system (GDAPS) which aims for installing apps dynamically (more or less) - but my main application does not know which plugin app is going to be installed. So adding an app to an array in settings.py does not work here. and doesn't have to.

I use this feature in GDAPS as well. GDAPS looks for the urlsmodule of each installed app and loads them dynamically, assembling all urlpatterns together. Unicorn could do that with components.

@adamghill
Copy link
Owner

adamghill commented Jun 9, 2021

Definitely not a technical problem to implement this. It was mostly a performance optimization (to skip checking every installed app) and to provide an "easy" on-ramp with the startunicorn management command (by defaulting to a unicorn app).

One question I have is whether this would this work for your use-case?

UNICORN = {
    ...
    "APPS": INSTALLED_APPS,
    ...
}

@adamghill adamghill self-assigned this Jun 9, 2021
@nerdoc
Copy link
Contributor Author

nerdoc commented Jun 10, 2021

"APPS": INSTALLED_APPS, - this would definitely do it. Performance shouldn't be an issue, as it is only done once at server start. Yes, the server starts a little bit slower, but who cares about that...?
I would make this even more easy: Why not per default search in apps' components modules instead of a "unicorn" app?
This would remove the awkwardly needed "unicorn" app and the adding of "unicorn" to INSTALLED_APPS. Many Django additions do that, even Django itself has the convention: search in the models module for models. Why not search in the components module for components? Would be even more django'ish :-)
You wouldn't even need an APPS setting: apps that don't have a components module, are ignored.

@nerdoc
Copy link
Contributor Author

nerdoc commented Jun 10, 2021

... and to provide an "easy" on-ramp with the startunicorn management command (by defaulting to a unicorn app).

Oh, and this could also be dont with a mgmt cmd. Sockpuppet does this, you just need to add a parameter to the command which specifies the app where the scaffolding has to be placed. I do that in GDAPS mgmt cmd too.

./manage.py startunicorn <my_app> <my_component>

could create

my_app
    (models.py)
    (views.py)
    (apps.py)
    (...)
    templates
        my_app
            component.py
    components
        my_component.html

Then you must call this with {% unicorn 'my_app/my_component' %}

@adamghill
Copy link
Owner

My performance optimization comment was because (currently) all apps are checked the first time a component is loaded (it happens here:

def get_locations(component_name):
). I have some caching around that section of code, but I could probably offset any potential performance hit by being a little smarter about how this works.

This would remove the awkwardly needed "unicorn" app and the adding of "unicorn" to INSTALLED_APPS
I completely agree with this. The unicorn app install step is a little confusing in the documentation. "Why do I need django_unicorn and unicorn?!"

I like this approach and it seems reasonably straight-forward to implement, and potentially less confusing for end-users. I'll try to whip up a PR when I get a chance and will attach it to this issue.

@adamghill adamghill changed the title Allow apps define components Look for Unicorn components in all INSTALLED_APPS by default Jun 10, 2021
@adamghill
Copy link
Owner

I started looking into how to get this working. Loading the components from INSTALLED_APPS is working in the attached PR.

I think I'm going to still default templates to a "unicorn" folder underneath the application's template folder. I think it makes sense to keep Unicorn templates separate by default. If someone really wants them somewhere else you can manually set template_name in the component view.

@adamghill
Copy link
Owner

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

No branches or pull requests

2 participants