Skip to content

feat: update tiled node insertion structure - #1669

Open
ZohebShaikh wants to merge 10 commits into
mainfrom
tiled-path-change
Open

ZohebShaikh wants to merge 10 commits into
mainfrom
tiled-path-change

Conversation

@ZohebShaikh

Copy link
Copy Markdown
Contributor

Stack created with GitHub Stacks CLI • Give Feedback 💬

@ZohebShaikh
ZohebShaikh added this pull request to stack #1670 September 11, 2026 10:44
@codecov

codecov Bot commented Sep 11, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.41%. Comparing base (f9af725) to head (068fbe1).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1669      +/-   ##
==========================================
+ Coverage   96.36%   96.41%   +0.05%     
==========================================
  Files          49       49              
  Lines        3686     3711      +25     
==========================================
+ Hits         3552     3578      +26     
+ Misses        134      133       -1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ZohebShaikh
ZohebShaikh force-pushed the tiled-path-change branch 2 times, most recently from 4414034 to abcd7ff Compare September 11, 2026 14:45
diamond-policies:
service: ghcr
resource: ghcr.io/diamondlightsource/authz-policy:0.0.24
resource: ghcr.io/zohebshaikh/authz-policy:0.0.25-alpha

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fajinyuan Here is the PR in authz that needs to be merged before this gets in.
Once merged in authz I will release and change here

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no description or context to guide reviewers on this related PR.

@ZohebShaikh
ZohebShaikh force-pushed the tiled-path-change branch 2 times, most recently from b5295b6 to cf72900 Compare September 11, 2026 15:35
@ZohebShaikh
ZohebShaikh force-pushed the tiled-path-change branch 2 times, most recently from 0a60f5b to a0df79b Compare September 11, 2026 15:49
@ZohebShaikh
ZohebShaikh marked this pull request as ready for review September 16, 2026 08:10
@ZohebShaikh
ZohebShaikh requested a review from a team as a code owner September 16, 2026 08:10

@tpoliaw tpoliaw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really know how the authorization stuff for tiled works but it looks plausible.

The amount of stuff going on in the begin_task is starting to look a bit suspect though. Can be a later PR but is it possible offload some of it to the tiled writer or client?

Comment thread src/blueapi/service/interface.py Outdated
Comment thread src/blueapi/utils/__init__.py Outdated
Comment thread src/blueapi/service/interface.py Outdated
Comment thread src/blueapi/service/interface.py Outdated
Comment thread src/blueapi/service/interface.py Outdated
Comment thread src/blueapi/service/interface.py Outdated
Comment thread tests/system_tests/test_blueapi_system.py Outdated
Base automatically changed from tiled-update to main October 6, 2026 13:59

@fajinyuan fajinyuan left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this change a mistake?

diamond-policies:
service: ghcr
resource: ghcr.io/diamondlightsource/authz-policy:0.0.24
resource: ghcr.io/zohebshaikh/authz-policy:0.0.25-alpha

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do you need change the resource path to a personal one?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is not enough information in referenced PR for me to understand the reason for this personalised path in URL

ZohebShaikh and others added 3 commits October 17, 2026 12:26
begin_task checked and opened each of beamline, proposal and visit in
turn, six requests before every run, and built a new tiled client (server
info plus a token) for every task. Look up beamline/proposal/visit in one
request, and only walk the levels when it is missing: create each, falling
back to opening it on a 409. Reuse one service-account client across
tasks; a client carrying a caller's pass-through headers is still made per
task.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Tiled in the system tests now runs ghcr.io/zohebshaikh/dls-tiled, which
ships the DLS access policy (dls_tiled.policy), instead of a copy of that
policy mounted into the stock tiled image; that copy and its unit tests
are removed, along with the tiled[minimal-server] dev dependency only they
used. OPA moves to authz-policy 0.0.27-alpha, which grants access at the
level it is held.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
edge-static-debug floated with every OPA build. Pin 1.21.2, the version
authz now builds and tests its policy with; the debug variant is kept
because entrypoint.sh needs a shell.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
0.0.28-alpha is built from authz 452bd95: the same decisions as
0.0.27-alpha, with the policy written for OPA 1.21.2 (string
interpolation, so it needs OPA 1.x to load).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

4 participants