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

Confirm if get_config_form() is optional or not #44

Closed
unbracketed opened this issue Apr 23, 2024 · 2 comments
Closed

Confirm if get_config_form() is optional or not #44

unbracketed opened this issue Apr 23, 2024 · 2 comments
Labels
bug Something isn't working documentation Improvements or additions to documentation

Comments

@unbracketed
Copy link

unbracketed commented Apr 23, 2024

While writing a "hello world" / simplest version of an enrichment plugin to get started with, I found the documentation regarding get_config_form slightly confusing:

The get_config_form() method can optionally be implemented

...sounds to me like the method is optional, but if one is not implemented the base class returns None resulting in the following error:

Traceback (most recent call last):
  File "/Users/brian/code/exploration/datasette/datasette/.venv/lib/python3.12/site-packages/datasette/app.py", line 1668, in route_path
    response = await view(request, send)
               ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/brian/code/exploration/datasette/datasette/.venv/lib/python3.12/site-packages/datasette/app.py", line 1861, in async_view_fn
    response = await async_call_with_supported_arguments(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/brian/code/exploration/datasette/datasette/.venv/lib/python3.12/site-packages/datasette/utils/__init__.py", line 1021, in async_call_with_supported_arguments
    return await fn(*call_with)
           ^^^^^^^^^^^^^^^^^^^^
  File "/Users/brian/code/exploration/datasette/datasette/.venv/lib/python3.12/site-packages/datasette_enrichments/views.py", line 77, in enrichment_view
    form = (
           ^
TypeError: 'NoneType' object is not callable

I'm not sure if this is considered a bug depending on what the expected behavior is. Does every enrichment need to present a form?

@simonw simonw added the bug Something isn't working label Apr 27, 2024
@simonw
Copy link
Collaborator

simonw commented Apr 27, 2024

Yes, definitely a bug, thanks. I need to think of an example of an enrichment that doesn't need a config form at all, if I can't think of one then maybe it should be required.

@simonw simonw changed the title Documentation improvement for enhancement plugin authors Confirm if get_config_form() is optional or not Apr 27, 2024
@simonw simonw added the documentation Improvements or additions to documentation label Apr 27, 2024
@simonw
Copy link
Collaborator

simonw commented Apr 27, 2024

For an enrichment plugin to not have get_config_form() implemented it would need to be able to do something useful to a filtered selection of table rows without any extra configuration.

Some ideas:

  • Back up those rows to some pre-configured location
  • Calculate a sha256 hash of the contents of all of the columns and stick that in a sha_256 column
  • Identify duplicate rows (not sure where I would put the identified duplicate information though, maybe just straight up delete rows that are exact matches for others?)

None of these are very exciting, but I'll spin up an automated test around the sha256 idea just to figure out how to close this issue.

@simonw simonw closed this as completed in f255651 Apr 27, 2024
simonw added a commit that referenced this issue Apr 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants