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

Custom collectors #98

Closed
wants to merge 2 commits into from
Closed

Conversation

wolflu05
Copy link
Contributor

This PR is a proposal for custom collectors. Those can be used to inject custom data into the collected django index or modify any existing collected data.

Please let me know if you like that approach or if you have any better idea. If you find it acceptable I'll add some short documentation around it. The main idea is to e.g. create a file anywhere in the django project like my_app/custom_collector.py with the following contents:

def collect_inventree_data(collector):
    # filter templates based on their name
    collector.templates = {
        name: value
        for name, value in collector.templates.items()
        if name.endswith("_base.html")
    }

    # add some custom templates
    collector.templates["my_test_template.html"] = {
        "path": "",
        "extends": None,
        "blocks": [],
        "context": {"hello": None}
    }

    # add custom global context
    collector.global_template_context["abc"] = None

And then specify the --custom-collector-module my_app.custom_collector.collect_inventree_data.

@krukas
Copy link
Contributor

krukas commented Mar 28, 2025

If collect_inventree_data is manageable code, you can add it the same way wagtail is added (https://github.com/fourdigits/django-template-lsp/blob/main/djlsp/scripts/django-collector.py#L558). And we need to spec out first how plugin support should look like.


There was also an issue for this kind of addition #86. I think it would be nice to have some plugin support, maybe in plugin/contrib folder or seperate package with the use the https://docs.python.org/3/library/importlib.metadata.html#entry-points for register the plugins.

Settings for plugin can be done the same way as https://github.com/python-lsp/python-lsp-server/blob/develop/CONFIGURATION.md

For the django collector the plugin a file or plain script the is used/injected by the django collector script, or called seperatly. I think that it would be nice if its used/injected by django collector script. Because plugins can update existing indexed data, like this wagtail function could be a plugin: https://github.com/fourdigits/django-template-lsp/blob/main/djlsp/scripts/django-collector.py#L55

@wolflu05
Copy link
Contributor Author

wolflu05 commented Mar 28, 2025

Hi thanks for your so quick response. I saw the wegtail collector, but I think my usecase is a bit different and don't makes sense to integrate into this repository. If you dont mind, let me explain it shortly:

InvenTree is an open source inventory management software. It supports generating labels (you can print and stick onto the inventory) or reports (like a purchase order delivery notes, ...) based on django templates that can be created by admin users on runtime. To make this template creating process easier for non-programmers we are building a custom web-editor based on monaco (screenshot of the previous version) that should integrate with the LSP. So we are not using the LSP for standard django application development.
Thats why we need to do a lot of modifications on the collected data e.g.

  • strip out all unnecessary templates from the collected data and only include the ones that should be used as base.
  • manually introspect the available context for these base templates based on the InvenTree source code
  • remove the URLs and some static files, they should not be shown to the user as they are unnecessary
  • remove some things from the global context

Later I like to look into also presenting live production data from the context to the user via the LSP in completions/hover, but not sure how that can be implemented, because I have to somehow interface with the LSP directly.

EDIT: Just found jedi.Interpreter while browsing through jedis capabilities. This seems to be very useful here for this. Maybe showing production context data can be integrated also using such a plugin system. So that would make sense to consider now when deciding on a plugin solution.


I really like the idea of using importlib metadata entrypoints for discovering custom collectors, havent thought about that. I was already thinking, that adding more and more CLI args feels very redundant and hard to maintain.

So do you want me to look into using importlib metadata entrypoints?
May you could explain a bit more in detail how you imagine a plugin file to look like in API? Should that be a class instead of a simple function that inherits from something provided by djlsp? I would really appreciate if you could take the time to make a small code example of how you imagine a custom collector plugin, so that we are aligned.

@krukas
Copy link
Contributor

krukas commented Mar 28, 2025

Thanks for the explanation, sounds like a fun project. Maybe the easiest solutions with less impact on code base add cli/init option to give a custom DJANGO_COLLECTOR_SCRIPT_PATH and just change this from constant to server attribute that is set with initialization_options

@wolflu05
Copy link
Contributor Author

Sorry, I do not fully understand what you mean. I actually liked the importlib entrypoints suggestion. And I feel like if we build something, that should be capable to be later extended. Such a simple script approch I proposed dont really seems to be extendable in the future with integration into the LSP. E.g. allow plugins to contribute actual data for the context with is used by the LSP and shown to the user.

I would really like to hear your opinion on how such a plugin interface may look like before I start implementing it.

@krukas
Copy link
Contributor

krukas commented Mar 28, 2025

I agree that in the long run a plugin systems would be more extendable. Only this takes some work to design an API for it and it will add complexity to the code base.

My suggestion is that we have an option to use your own django collector instead of ours. This is only a few lines to change and I think will solve you problem now. And then we can take some time to spec out the way plugins/contrib should work.

@wolflu05
Copy link
Contributor Author

You're totally right, that takes time.

But how do you think that my custom collector script should work then? If I just call the builtin script from mine and parse the returned json. How can I access the django application now? Would that mean I would need to duplicate the code that bootstraps the django application, which also means it has to initialize again => I have to wait 20s now.

@krukas
Copy link
Contributor

krukas commented Mar 28, 2025

Think easiest is to make a branch from this repo add a function like wagtail and do all clean-up/changes there. This should not change the current code and make rebasing on it pretty easy.

@wolflu05
Copy link
Contributor Author

Mh ok, maintinaing a fork of this repo seems to much effort, then I'll try to build a simple script that just applies those 3 lines which runs my custom function after the collector has finished, before starting the LSP. But I can understand, this seems like a rare edgecase usage of this LSP.
I am already looking forward to a full plugin support then. Should I create an issue to track that?

@krukas
Copy link
Contributor

krukas commented Mar 31, 2025

For your fork you only have to maintain the Django collector script, I can understand that you don't like that. And yes you can create an issue to track plugin support.

@wolflu05 wolflu05 closed this Mar 31, 2025
@wolflu05 wolflu05 mentioned this pull request Mar 31, 2025
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.

2 participants