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

Move permissions, allow blocks, canned queries and more out of metadata.yaml and into datasette.yaml #2191

Merged
merged 6 commits into from
Oct 12, 2023

Conversation

asg017
Copy link
Collaborator

@asg017 asg017 commented Sep 18, 2023

The PR moves the following fields from metadata.yaml to datasette.yaml:

permissions
allow
allow_sql
queries
extra_css_urls
extra_js_urls

This is a significant breaking change that users will need to upgrade their metadata.yaml files for. But the format/locations are similar to the previous version, so it shouldn't be too difficult to upgrade.

One note: I'm still working on the Configuration docs, specifically the "reference" section. Though it's pretty small, the rest of read to review

@codecov
Copy link

codecov bot commented Sep 18, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (6ed7908) 92.69% compared to head (0135e7c) 92.68%.
Report is 14 commits behind head on main.

❗ Current head 0135e7c differs from pull request most recent head 18b48f8. Consider uploading reports for the commit 18b48f8 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2191      +/-   ##
==========================================
- Coverage   92.69%   92.68%   -0.02%     
==========================================
  Files          40       40              
  Lines        6039     6042       +3     
==========================================
+ Hits         5598     5600       +2     
- Misses        441      442       +1     
Files Coverage Δ
datasette/app.py 94.09% <100.00%> (-0.11%) ⬇️
datasette/default_permissions.py 97.36% <100.00%> (+0.01%) ⬆️

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@simonw
Copy link
Owner

simonw commented Sep 20, 2023

I tested this locally for permissions like this. datasette.yml:

databases:
  content:
    allow:
      id: root

Started Datasette like this:

datasette --root content.db pottery2.db -c datasette.yml

As root I could see this (note the padlock):

CleanShot 2023-09-20 at 15 17 02@2x

http://127.0.0.1:8001/-/metadata returned {} showing that the permissions must have come from the config file instead.

@simonw
Copy link
Owner

simonw commented Sep 20, 2023

This is one of the most interesting illustrative examples in the new code:

datasette/tests/fixtures.py

Lines 301 to 349 in f7bdedf

CONFIG = {
"plugins": {
"name-of-plugin": {"depth": "root"},
"env-plugin": {"foo": {"$env": "FOO_ENV"}},
"env-plugin-list": [{"in_a_list": {"$env": "FOO_ENV"}}],
"file-plugin": {"foo": {"$file": TEMP_PLUGIN_SECRET_FILE}},
},
"databases": {
"fixtures": {
"plugins": {"name-of-plugin": {"depth": "database"}},
"tables": {
"simple_primary_key": {
"plugins": {
"name-of-plugin": {
"depth": "table",
"special": "this-is-simple_primary_key",
}
},
},
"sortable": {
"plugins": {"name-of-plugin": {"depth": "table"}},
},
},
"queries": {
"𝐜𝐢𝐭𝐢𝐞𝐬": "select id, name from facet_cities order by id limit 1;",
"pragma_cache_size": "PRAGMA cache_size;",
"magic_parameters": {
"sql": "select :_header_user_agent as user_agent, :_now_datetime_utc as datetime",
},
"neighborhood_search": {
"sql": textwrap.dedent(
"""
select _neighborhood, facet_cities.name, state
from facetable
join facet_cities
on facetable._city_id = facet_cities.id
where _neighborhood like '%' || :text || '%'
order by _neighborhood;
"""
),
"title": "Search neighborhoods",
"description_html": "<b>Demonstrating</b> simple like search",
"fragment": "fragment-goes-here",
"hide_sql": True,
},
},
}
},
}

Interesting to note that it now has canned queries in it, which include this bit:

datasette/tests/fixtures.py

Lines 341 to 342 in f7bdedf

"title": "Search neighborhoods",
"description_html": "<b>Demonstrating</b> simple like search",

It looks like metadata, but in this case it's configuration. That blur between metadata and configuration at the canned query level still feels a little bit odd to me, but I still think we're going in the right direction with it.

Also interesting, from that same file:

datasette/tests/fixtures.py

Lines 351 to 399 in f7bdedf

METADATA = {
"title": "Datasette Fixtures",
"description_html": 'An example SQLite database demonstrating Datasette. <a href="/login-as-root">Sign in as root user</a>',
"license": "Apache License 2.0",
"license_url": "https://github.com/simonw/datasette/blob/main/LICENSE",
"source": "tests/fixtures.py",
"source_url": "https://github.com/simonw/datasette/blob/main/tests/fixtures.py",
"about": "About Datasette",
"about_url": "https://github.com/simonw/datasette",
"extra_css_urls": ["/static/extra-css-urls.css"],
"databases": {
"fixtures": {
"description": "Test tables description",
"tables": {
"simple_primary_key": {
"description_html": "Simple <em>primary</em> key",
"title": "This <em>HTML</em> is escaped",
},
"sortable": {
"sortable_columns": [
"sortable",
"sortable_with_nulls",
"sortable_with_nulls_2",
"text",
],
},
"no_primary_key": {"sortable_columns": [], "hidden": True},
"units": {"units": {"distance": "m", "frequency": "Hz"}},
"primary_key_multiple_columns_explicit_label": {
"label_column": "content2"
},
"simple_view": {"sortable_columns": ["content"]},
"searchable_view_configured_by_metadata": {
"fts_table": "searchable_fts",
"fts_pk": "pk",
},
"roadside_attractions": {
"columns": {
"name": "The name of the attraction",
"address": "The street address for the attraction",
}
},
"attraction_characteristic": {"sort_desc": "pk"},
"facet_cities": {"sort": "name"},
"paginated_view": {"size": 25},
},
}
},
}

There are a few things in that metadata block that are arguably configuration, not metadata - for example:

"extra_css_urls": ["/static/extra-css-urls.css"],

I think extra_css_urls is definitely configuration, not metadata.

datasette/tests/fixtures.py

Lines 369 to 395 in f7bdedf

"sortable": {
"sortable_columns": [
"sortable",
"sortable_with_nulls",
"sortable_with_nulls_2",
"text",
],
},
"no_primary_key": {"sortable_columns": [], "hidden": True},
"units": {"units": {"distance": "m", "frequency": "Hz"}},
"primary_key_multiple_columns_explicit_label": {
"label_column": "content2"
},
"simple_view": {"sortable_columns": ["content"]},
"searchable_view_configured_by_metadata": {
"fts_table": "searchable_fts",
"fts_pk": "pk",
},
"roadside_attractions": {
"columns": {
"name": "The name of the attraction",
"address": "The street address for the attraction",
}
},
"attraction_characteristic": {"sort_desc": "pk"},
"facet_cities": {"sort": "name"},
"paginated_view": {"size": 25},

Most of that stuff is arguably configuration too, with the exception of the roadside_attractions.columns bit which is metadata about those columns.

@simonw
Copy link
Owner

simonw commented Sep 20, 2023

The {"units": {"distance": "m", "frequency": "Hz"}} bit is for the units feature which I've half-disabled already and would like to remove before 1.0, though ideally turning that functionality into a plugin instead (if I can figure out how to do that).

@asg017 asg017 changed the title DRAFT: Move permissions and allow blocks out of metadata.yaml and into datasette.yaml DRAFT: Move permissions, allow blocks, and canned queries out of metadata.yaml and into datasette.yaml Sep 29, 2023
@asg017 asg017 changed the title DRAFT: Move permissions, allow blocks, and canned queries out of metadata.yaml and into datasette.yaml DRAFT: Move permissions, allow blocks, canned queries and more out of metadata.yaml and into datasette.yaml Oct 5, 2023
@asg017 asg017 marked this pull request as ready for review October 5, 2023 19:31
@simonw simonw changed the title DRAFT: Move permissions, allow blocks, canned queries and more out of metadata.yaml and into datasette.yaml Move permissions, allow blocks, canned queries and more out of metadata.yaml and into datasette.yaml Oct 12, 2023
@simonw simonw merged commit 35deaab into simonw:main Oct 12, 2023
8 checks passed
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