DRAFT: A new plugin hook for dynamic metadata by brandonrobertz · Pull Request #1368 · simonw/datasette

brandonrobertz · 2021-06-07T18:56:00Z

Note that this is a WORK IN PROGRESS!

This PR adds the following plugin hook:

get_metadata(
  datasette=self, key=key, database=database, table=table,
  fallback=fallback
)

This gets called when we're building our metdata for the rest of the system to use. Datasette merges whatever the plugins return with any local metadata (from metadata.yml/yaml/json) allowing for a live-editable dynamic Datasette. A major design consideration is this: should Datasette perform the metadata merge? Or should Datasette allow plugins to perform any modifications themselves?

As a security precation, local meta is not overwritable by plugin hooks. The workflow for transitioning to live-meta would be to load the plugin with the full metadata.yaml and save. Then remove the parts of the metadata that you want to be able to change from the file.

I have a WIP dynamic configuration plugin here, for reference: https://github.com/next-LI/datasette-live-config/

brandonrobertz · 2021-06-07T18:59:47Z

Note that if we went with a "update_metadata" hook, the hook signature would look something like this (it would return nothing):

update_metadata(
  datasette=self, metadata=metadata, key=key, database=database, table=table,
  fallback=fallback
)

The Datasette function _metadata_recursive_update(self, orig, updated) would disappear into the plugins. Doing this, though, we'd lose the easy ability to make the local metadata.yaml immutable (since we'd no longer have the recursive update).

simonw · 2021-06-21T16:07:06Z

A few tests failed - Black, the test that checks the docs mention the new hook - the most interesting failing test looks like this one:

            updated_metadata["databases"]["fixtures"]["queries"]["magic_parameters"][
                "allow"
            ] = (allow if "query" in permissions else deny)
>           cascade_app_client.ds._metadata = updated_metadata
E           AttributeError: can't set attribute

From

datasette/tests/test_permissions.py

Lines 439 to 467 in 0a7621f

    
           def test_permissions_cascade(cascade_app_client, path, permissions, expected_status): 
        
               """Test that e.g. having view-table but NOT view-database lets you view table page, etc""" 
        
               allow = {"id": "*"} 
        
               deny = {} 
        
               previous_metadata = cascade_app_client.ds._metadata 
        
               updated_metadata = copy.deepcopy(previous_metadata) 
        
               actor = {"id": "test"} 
        
               if "download" in permissions: 
        
                   actor["can_download"] = 1 
        
               try: 
        
                   # Set up the different allow blocks 
        
                   updated_metadata["allow"] = allow if "instance" in permissions else deny 
        
                   updated_metadata["databases"]["fixtures"]["allow"] = ( 
        
                       allow if "database" in permissions else deny 
        
                   ) 
        
                   updated_metadata["databases"]["fixtures"]["tables"]["binary_data"] = { 
        
                       "allow": (allow if "table" in permissions else deny) 
        
                   } 
        
                   updated_metadata["databases"]["fixtures"]["queries"]["magic_parameters"][ 
        
                       "allow" 
        
                   ] = (allow if "query" in permissions else deny) 
        
                   cascade_app_client.ds._metadata = updated_metadata 
        
                   response = cascade_app_client.get( 
        
                       path, 
        
                       cookies={"ds_actor": cascade_app_client.actor_cookie(actor)}, 
        
                   ) 
        
                   assert expected_status == response.status 
        
               finally: 
        
                   cascade_app_client.ds._metadata = previous_metadata

This test is directly manipulating _metadata purely for the purposes of simulating different permissions - I think updating it to manipulate _local_metadata instead would fix that.

simonw · 2021-06-21T16:09:53Z

datasette/app.py

+        # TODO: change this to update_metadata, pass meta back allow the
+        # plugins to mutate it?


Let's ditch the TODO comments before we merge the PR - they can live in PR comments the issue conversation instead.

simonw · 2021-06-21T16:10:11Z

datasette/app.py

+        # TODO: get full data for here, we need to work down to tables from that
+        # as opposed to simply returning table data above?


I don't fully understand this comment

simonw · 2021-06-21T16:10:53Z

datasette/app.py

+    @property
+    def _metadata(self):
+        return self.metadata()
+


If this is for backwards compatibility I don't think it's necessary - _metadata has never been part of Datasette's public documented API.

brandonrobertz · 2021-06-21T17:11:37Z

If this is a concept ACK then I will move onto fixing the tests (adding new ones) and updating the documentation for the new plugin hook.

simonw · 2021-06-23T19:32:01Z

Yes, let's move ahead with getting this into an alpha.

The following hook is added: get_metadata( datasette=self, key=key, database=database, table=table, fallback=fallback ) This gets called when we're building our metdata for the rest of the system to use. We merge whatever the plugins return with any local metadata (from metadata.yml/yaml/json) allowing for a live-editable dynamic Datasette. As a security precation, local meta is *not* overwritable by plugin hooks. The workflow for transitioning to live-meta would be to load the plugin with the full metadata.yaml and save. Then remove the parts of the metadata that you want to be able to change from the file.

This avoids the nasty "RuntimeError: OrderedDict mutated during iteration" error that randomly happens when a plugin adds a new database to Datasette, using `add_database`. This change makes the add and remove database functions more expensive, but it prevents the random explosion race conditions that make for confusing user experience when importing live databases.

simonw · 2021-06-26T22:23:57Z

The only test failure is Black. I'm going to merge this and then reformat.

simonw reviewed Jun 21, 2021

View reviewed changes

brandonrobertz force-pushed the main branch from ea8bf04 to d389b79 Compare June 25, 2021 03:39

brandonrobertz added 2 commits June 25, 2021 18:51

brandonrobertz force-pushed the main branch 2 times, most recently from ddad885 to b191bb6 Compare June 26, 2021 05:20

Fix: tests, style + add initial metadata hook test

84fe80c

brandonrobertz force-pushed the main branch from b191bb6 to 84fe80c Compare June 26, 2021 05:23

simonw merged commit baf986c into simonw:main Jun 26, 2021

simonw added a commit that referenced this pull request Jun 26, 2021

Applied Black, refs #1368

05a312c

simonw mentioned this pull request Jun 26, 2021

Plugin hook for dynamic metadata #1384

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

DRAFT: A new plugin hook for dynamic metadata#1368

DRAFT: A new plugin hook for dynamic metadata#1368
simonw merged 3 commits intosimonw:mainfrom
next-LI:main

brandonrobertz commented Jun 7, 2021 •

edited

Loading

Uh oh!

brandonrobertz commented Jun 7, 2021 •

edited

Loading

Uh oh!

simonw commented Jun 21, 2021 •

edited

Loading

Uh oh!

simonw Jun 21, 2021

Uh oh!

simonw Jun 21, 2021

Uh oh!

simonw Jun 21, 2021

Uh oh!

brandonrobertz commented Jun 21, 2021

Uh oh!

simonw commented Jun 23, 2021

Uh oh!

simonw commented Jun 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		# TODO: change this to update_metadata, pass meta back allow the
		# plugins to mutate it?

		# TODO: get full data for here, we need to work down to tables from that
		# as opposed to simply returning table data above?

Uh oh!

Conversation

brandonrobertz commented Jun 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandonrobertz commented Jun 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonw commented Jun 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

simonw Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

simonw Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

simonw Jun 21, 2021

Choose a reason for hiding this comment

Uh oh!

brandonrobertz commented Jun 21, 2021

Uh oh!

simonw commented Jun 23, 2021

Uh oh!

simonw commented Jun 26, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

brandonrobertz commented Jun 7, 2021 •

edited

Loading

brandonrobertz commented Jun 7, 2021 •

edited

Loading

simonw commented Jun 21, 2021 •

edited

Loading