Skip to content

Add a new Inherited access control policy - #1393

Open
dylanmcreynolds wants to merge 85 commits into
bluesky:mainfrom
als-computing:inherited_access_control
Open

Add a new Inherited access control policy#1393
dylanmcreynolds wants to merge 85 commits into
bluesky:mainfrom
als-computing:inherited_access_control

Conversation

@dylanmcreynolds

Copy link
Copy Markdown
Contributor

Checklist

  • Add a Changelog entry
  • Add the ticket number which this PR closes to the comment section

The default TagAccessPolicy requires that every node in the tree be tagged with access_tags. I think this creates a lot of future maintenance. If I were to try and change access for container of, say, proposals, I would have to make sure to surgically change the access_tag of every node under the proposal container. I'd rather just change the proposal container.

We also use tiled for storing processed data. The current setup puts a lot of responsibility on applications writing to tiled to get know how to tag every node that it writes. Using inheritance eases this.

Introduces InheritedTagAccessPolicy, a new access policy that extends TagBasedAccessPolicy by walking the node hierarchy when a node has no access tags of its own. Instead of defaulting to no access, it looks up the nodes_closure table to find the nearest tagged ancestor and applies that ancestor's access control rules. It ends at the first ancestor node that has at least one tag and uses that.

Also adds AccessBlobInheritedFilter to support filtering nodes by inherited access, and includes tests covering inherited access scenarios.

@checkmarx-gh-ast-us-povs

Copy link
Copy Markdown

Logo
Checkmarx One – Scan Summary & Detailsb6c866d4-ad42-4681-812d-a9db55f1e772

Great job! No new security vulnerabilities introduced in this pull request


Communicate with Checkmarx by submitting a PR comment with @Checkmarx followed by one of the supported commands. Learn about the supported commands here.

@dylanmcreynolds dylanmcreynolds changed the title A new Inherited access control policy Add a new Inherited access control policy May 24, 2026
@dylanmcreynolds
dylanmcreynolds requested a review from nmaytan June 5, 2026 13:51
@danielballan

Copy link
Copy Markdown
Member

Notes from in-person discussion on June 5

  • @nmaytan will review and approve @dylanmcreynolds' PR.
  • We will merge that with its current scope.
  • Scope for future PRs:
    • Tiled client should be able to ask, "What tags will I inherit if I create a child node here?"
    • Tiled client's tree command should be able to display tags on each line
    • Refactor access_blob to access_tags (either array of strings or many-to-many association to a separate table)
    • default_access_tags distinct from access_tags
      • If you have a container that has some access on it that might not necessarily be the access you want inherit by something that's inside of it. The access for something inside the container should be a different set of tags.
      • This is a way of giving broader access on contents than you have on the root, just as users have write access to the contents of a proposal directory but not to the directory itself.
      • We are not enamored with the named default_; let's workshop that with Claude.
      • Explore the consequence of this rule: just as with POSIX ACLs, if default_accesss_tags is not set, it's not a candidate for inheritance.
      • In the NSLS2 migration, un-tag everything under each BlueskyRun and update default_access_tags on them.
  • At ALS, TiledWriter/user can easily create /smi/raw/prop-12345. All scans for the proposal go in there.
  • Connecting AuthZ to the graph-of-links work:
    • For now, as entities always refer to nodes in Tiled, we don't need access tags on entities themselves. When, in the future, we extend entities to refer to external resources, we'll need to revisit this.
    • Links will have access tags on them.
    • Access to a link is determined by access to the link itself and both linked nodes.

@danielballan

Copy link
Copy Markdown
Member

Next steps:

  • @dylanmcreynolds will consolidate the "inheritance" code into the base class, rather than definite a separate policy in a subclass.
  • @nmaytan will review this PR

genematx and others added 18 commits August 12, 2026 13:03
* ENH: Add conversion from slice to shunk indices

* ENH: method to compute chunk indices for block

* ENH: Simplify and add build_nested_grid method

* MNT: changelog
Route stamina retry log messages to the tiled.client logger at DEBUG
level instead of the stamina logger at WARNING. This means retry
messages (e.g. 'Scheduled retry in 0.44 seconds due to ReadTimeout')
are suppressed in normal use and only appear when the user explicitly
calls show_logs().
* TST: concurrent requests limit

* ENH: set limits on concurrent requests

* ENH: increase the default number of concurrent connections

* MNT: changelog

* ENH: add max_connections options to client constructors

* FIX: pickling errors and client constructors
* TST: concurrent requests limit

* ENH: set limits on concurrent requests

* ENH: increase the default number of concurrent connections

* MNT: changelog

* ENH: add max_connections options to client constructors

* FIX: pickling errors and client constructors

* ENH: progress bar for Dask tasks

* WIP: fixing stamina logs

* FIX: disable stamina default hook, route tiled retries to tiled.client only

- Remove stamina's built-in LoggingOnRetryHook via set_on_retry_hooks([])
  so 'stamina.retry_scheduled' WARNING messages never appear
- Tiled's own retries are logged via _LoggingAttempt to tiled.client at
  DEBUG (visible only after show_logs(), silent by default)
- show_logs()/hide_logs() only toggle the tiled.client logger — no
  manipulation of the stamina logger level
- Other libraries using stamina are completely unaffected (their own
  hooks, if registered, still fire normally)
- Updated tests to verify all three invariants

* FIX: progress bar total counts actual chunks/partitions, not dask graph size

len(__dask_graph__()) includes internal dask tasks (fusion, concatenation)
which don't correspond to fetch calls. Use math.prod(len(c) for c in chunks)
for arrays and ddf.npartitions for dataframes to get the true fetch count.

* ENH: add show_progress option to Context and TILED_SHOW_PROGRESS env var

Progress bar is now opt-in (default False). Can be enabled via:
- Context(..., show_progress=True)
- from_uri(..., show_progress=True)
- TILED_SHOW_PROGRESS=1 environment variable

This ensures tiled never shows unexpected progress bars when used as
a library dependency inside another application.

* FIX: only render progress bar when session is interactive

show_progress=True (or TILED_SHOW_PROGRESS=1) now expresses intent;
the bar is still suppressed in non-interactive contexts (scripts,
services) to avoid broken terminal output.

* ENH: add streaming byte-level progress bar for single-request downloads

Adds _streaming_fetch() helper that uses httpx streaming with a Rich
progress bar showing bytes downloaded, transfer speed, and ETA. The bar
is displayed only when:
- context.show_progress is True
- the session is interactive
- the response is >= 10 MiB

Applied to:
- export_util() (all .export() calls on array, dataframe, sparse,
  awkward, and container clients)
- SparseClient.read()
- AwkwardClient.read()

For smaller responses or non-interactive sessions, falls back to the
standard buffered request with no visual overhead.

* ENH: add unified progress bar for DatasetClient.read()

DatasetClient.read() now wraps .load() with tracking_progress,
counting total fetch tasks (chunks) across all dask-backed variables
in the dataset. This provides a single progress bar for the entire
dataset load rather than per-variable silence.

* ENH: add unified progress bar for CompositeClient.read()

CompositeClient.read() now counts total fetch tasks (array chunks,
table partitions, awkward buffers) across all constituent parts and
wraps the entire read with tracking_progress.

Also makes tracking_progress a no-op when an outer progress bar is
already active (prevents nested bars when CompositeClient calls
ArrayClient.read() internally).

* FIX: correct progress bar total counting and remove debug print

- DatasetClient.read(): only count chunks from direct-fetch variables,
  not wide-table variables (which use dask.delayed and never advance
  the progress bar)
- CompositeClient.read(): use structure metadata from contents dict
  instead of making extra HTTP requests to count table columns
- Remove stray print('Loading data from Tiled...') debug output

* TST: add tests for progress bar infrastructure

Tests cover:
- tracking_progress sets/clears _progress_state correctly
- tracking_progress is a no-op when show_progress=False
- tracking_progress nesting defers to outer bar
- _streaming_fetch returns correct bytes (non-streaming path)
- _streaming_fetch writes to file correctly
- TILED_SHOW_PROGRESS env var parsing (1/0/true/invalid)
- Explicit show_progress overrides env var

* ENH: default show_progress to True (opt-out for library consumers)

Progress bars are now shown by default in interactive sessions.
Libraries embedding tiled can disable with:
- Context(..., show_progress=False)
- TILED_SHOW_PROGRESS=0 environment variable

* REFACTOR: inline _read_inner back into CompositeClient.read()

Remove the separate _read_inner method; use tracking_progress as a
context manager directly within read() wrapping only the fetch loop.

* MNT: format and lint

* ENH: respect 429 Too Many Requests with Retry-After header (bluesky#1398)

The client now retries on HTTP 429 responses, respecting the
Retry-After header value as the wait time before the next attempt.
This works with load balancers that throttle clients.

Changes:
- handle_error() lets 429 propagate as plain HTTPStatusError
  (not converted to ClientError) so stamina can retry it
- should_retry() returns the Retry-After value as a float for 429,
  which stamina uses as the retry delay (overriding default backoff)
- If no Retry-After header is present, falls back to default backoff

Requires stamina >= 25.1.0 (predicate-returns-float feature).

* TST: add integration tests for 429 retry behavior

- test_retry_context_retries_on_429: verifies retry loop completes
  after 429s stop
- test_retry_context_logs_429_retry: verifies retry is logged at DEBUG
- test_429_retry_with_real_server: end-to-end test with mock httpx
  transport returning 429 twice then 200

* REVERT: remove streaming byte-level progress bar

Remove _streaming_fetch and all streaming-related changes. Keep only
the chunk-count progress bar (tracking_progress) for dask-based reads.

Reverted files:
- utils.py: removed _streaming_fetch, _PROGRESS_THRESHOLD, import io;
  restored export_util to original signature
- sparse.py: restored buffered read with retry_context
- awkward.py: restored buffered read with retry_context
- array.py, dataframe.py, container.py: removed context= from export_util
- test_client.py: removed _streaming_fetch tests

* FIX: use .get() for Content-Type header check in handle_error

Prevents potential KeyError when a proxy returns a response without
a Content-Type header. Also adds clarifying comment to __setstate__
explaining why show_progress is intentionally False for unpickled
contexts (dask workers).

* WIP

* FIX: spinner for retries

* FIX: mnor bug fixes

* MNT: changelog

* FIX: Interactive client in Jupyter notebook

* MNT: refactor private methods

* FIX: progress bar in Jupyter notebooks
* ENH: ensure row ordering in SQLAdapter

* MNT: changelog

* fix: unregister storage after dispose to allow postgres DROP DATABASE in teardown

* MNT: format and lint

* fix: terminate lingering postgres sessions before DROP DATABASE in temp_postgres

* MNT: format

* test: update order_by tests to match new order_by_args/primary_key API; add desc, multi-column, uniqueness tests

* refactor: accept only canonical types for order_by_args and primary_key; remove str shorthand normalization

* MNT: format and lint

* MNT: changelog

* ENH: validate parameters against non-allowed column dtypes

* TST: add tests for Postgres
* no retry for non supported protocols

* add local protocol error

* add to changelog and reformat test for pre commit hook
* MNT: refactor awkward adapter

* MNT: refactor DirectoryContainer

* MNT: changelog

* MNT: remove irrelevant code

* FIX: from_catalog classmethod

* Update tiled/storage.py

Co-authored-by: Connor Boyle <connor@cjboyle.ca>

* FIX: typo

* ENH: separate in-memory and storage adapters

* MNT: rename awk_form to awkward_form

---------

Co-authored-by: Connor Boyle <connor@cjboyle.ca>
* check for array of arrays and convert to ndarray

* Add ragged dependency

* From SQLAdapter, test Array-, Ragged-, then AwkwardAdapter

* Test returned adapters, without nullable data types

* remove normalize_chunks from ragged adapter

* Add schema tests for irregular arrays

* No need to test every datatype, already done elsewhere

* write + read full ragged arrays

* fix lack of `read()`

* add more complexity to tests

* test simple to complex arrays

* Update structure to store offsets

* fix exit clause logic

* fix parameter order

* test ragged structure and utilities

* wip: writing/reading full arrays from flattened .npy files working

* update tmp location for JSON export

* Add size field to structure, to make RaggedClient closer to ArrayClient.

* wip: reading sliced data (tests commented)

* Fix "Self" for python<=3.10

* wip: slicing

* don't ignore ListArrays, they have feelings too!

* slicing finished

* speed up tests, no need to restart for different slices

* formatting

* remove unused imports

* add sanity check, and comment for clarity

* rename to differentiate from actual ragged array

* fix awkward isn't smart enough to convert ndarray of ndarrays

* test reading jagged data out of SQL to the client

* Add slicing to SQL tests

* cleanup some unused or duplicated code

* move static function out of class

* remove unused function

* some cleanup and WIP sliced-exports

* mark sliced-export tests of scalar values with XFAIL

* update comments

* raise HTTP 406 code when exporting single scalar value

* make JSON the default format for unknown HTTP clients

* add changelog entry

* Add docs for using ragged

* generalization -> specialization

* try reducing uniform list-of-lists

* Switch to parquet for file storage and add data partitioning support

* update docs metadata

* increase testability

* fix package name typo

* skip blosc2 compression to fix error with 32-bit dtypes

* refactor to remove offsets from RaggedStructure

* removed dask-awkward, datatypes broken

* fix weird dtype/bit conversion

* reduce loop nesting

* remove offsets from documented structure output

* no need to omit blosc2 explicitly with application/zip

* fix incorrect partition upper bound

* add docstrings, and some cleanup

* test with numpy, awkward, and list-of-lists as input

* adapt NDBlock similar to other usages

* moved CHANGELOG entry to "unreleased"

* provide correct `nbytes` size from Awkward

Co-authored-by: Copilot <copilot@github.com>

* ENH: add alembic migration

* ENH: add alembic migration

* TST: revert changes to existing tests

* TST: test sql arrays passing

* TST: use module-scoped fixtures

* TST: add comment

* MNT: comments

* MNT: remove noqa: SLF001

* Update tests/test_ragged.py

Co-authored-by: Connor Boyle <connor@cjboyle.ca>

* import make_ragged_array in tests

* MNT: refactor awkward adapter

* MNT: refactor DirectoryContainer

* MNT: changelog

* MNT: remove irrelevant code

* FIX: from_catalog classmethod

* remove ambiguous nbytes altogether, as it isn't used anywhere.

* wip: refactor 'partitions' to 'chunks'

* cleanup determining shape of chunks/blocks

* fixed block slice expansion

* WIP: ragged structure

* WIP: writing and reading with RaggedSQLAdapter

* WIP:

* TST: expand test cases for ragged

* TST: adapter writing roundrip tests

* ENH: ensure row ordering in SQLAdapter

* MNT: changelog

* fix: unregister storage after dispose to allow postgres DROP DATABASE in teardown

* MNT: format and lint

* TST: roundrip tests for adapter passing

* fix: terminate lingering postgres sessions before DROP DATABASE in temp_postgres

* MNT: format

* TST: chunking tests passing

* test: update order_by tests to match new order_by_args/primary_key API; add desc, multi-column, uniqueness tests

* refactor: accept only canonical types for order_by_args and primary_key; remove str shorthand normalization

* MNT: format and lint

* MNT: changelog

* ENH: validate parameters against non-allowed column dtypes

* TST: add tests for Postgres

* TST: all tests passing

* MNT: cleanup the adapters code

* MNT: clean up with pre-commit

* FIX: potential concurrency issues

* TST: expand tests

* ENH: remove read_block for ragged

* ENH: enable ragged arrays in composite structures

* ENH: streaming ragged arrays

* ENH: only load necessary slices from SQL

* ENH: allow regularization in make_ragged_array

* ENH: add utility to convert ragged arrays to dense

* TST: read ragged arrays into xarray

* ENH: support empty ragged arrays

* MNT: reachability comment

* fix indent

* fix indent again

* fix indent once more

* MNT: format and lint

* TST: fix tests on py3.10 and windows

* TST: tolerate Windows file-handle cleanup lag in test_sync

Replace the post-yield dispose/unregister dance with
TemporaryDirectory(ignore_cleanup_errors=True) (Python 3.10+).

The previous approach assumed storage.dispose() synchronously releases
the duckdb file handle, but on Windows ADBC's handle can outlive the
SQLAlchemy pool dispose, producing WinError 32 during rmtree. The
stdlib idiom mirrors the lenient cleanup pytest's tmpdir/tmp_path
fixtures provide elsewhere in the test suite (test_writing.py,
conftest.py:403).

* MNT: rename awk_form to awkward_form

* FIX: slicing with [0]

* TST: tests for RaggedAdapter protocol

---------

Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Eugene M. <ymatviych@bnl.gov>
dylanmcreynolds and others added 26 commits August 12, 2026 13:06
* BUG: point landing page GraphQL link at /api/graphql

* MNT: remove unused graph.core module

* BUG: create and drop namespaces table in graph migration

* ENH: provision graph tables through the catalog schema

Declare the entities, links, and namespaces tables on the catalog
Base.metadata so fresh databases create them via initialize_database,
matching the migration used for existing databases. The graph store no
longer creates its own schema at runtime and reuses the catalog nodes
table for node id resolution.

* ENH: preload a graph-specific query in the GraphiQL editor

* STY: tidy imports in graph table provisioning

* DOC: minor fixes

* MNT: rename graph.tables module to graph.orm

Match the catalog's naming convention (tiled.catalog.orm), where SQLAlchemy
table definitions on Base.metadata live in an orm module. No behavior change.

* ENH: limit GraphQL query depth on the graph endpoint

The entity/link graph is recursively traversable (Entity.outgoingLinks ->
Link.object -> Entity.outgoingLinks -> ...), so an unbounded nested query
could force arbitrarily deep and expensive resolution. Add a QueryDepthLimiter
(max depth 10) to the schema. Introspection queries are exempt, so the
GraphiQL Docs panel is unaffected.

* PERF: create graph links via foreign-key constraint instead of pre-checks

create_link previously issued two SELECTs to confirm the subject and object
entities exist before inserting, duplicating work the schema layer already
does and leaving a window between the checks and the insert. Insert directly
and rely on the subject_id/object_id foreign keys (SQLite enforces these too,
via the shared pool's PRAGMA foreign_keys=ON). Only on a constraint violation
does it look up which endpoint is missing, so the success path is a single
atomic INSERT while error messages are unchanged.

* DOC: remove stale JSON-LD import/export references
Revert the coalescing of native HDF5 chunks (READ_BATCH_BYTES-bounded
blocks), parallel spec reads, and resource-cache wrapping of the lazy
Dask graph introduced for the HDF5 adapter in bluesky#1463, restoring
tiled/adapters/hdf5.py and the coupled tests in tests/test_hdf5.py to
their pre-bluesky#1463 state. The coalescing made a single-frame read pull up to
a 1 GiB block, since a block is also the minimum a partial read fetches.

The TIFF and file-sequence adapter improvements from bluesky#1463 (parallel,
memory-bounded reads and lazy asset resolution) are left intact.
@dylanmcreynolds
dylanmcreynolds force-pushed the inherited_access_control branch 3 times, most recently from c8af3a9 to 7800929 Compare August 13, 2026 00:28
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.